From 7520ed457142cd19d50dfca74debea646ff7beba Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Fri, 24 May 2024 11:06:41 -0400 Subject: [PATCH 01/42] Initial simulate implementation (correct error code, no metadata) --- src/xrpld/app/tx/detail/Transactor.cpp | 27 ++-- src/xrpld/ledger/ApplyView.h | 4 + src/xrpld/rpc/detail/Handler.cpp | 1 + src/xrpld/rpc/handlers/Handlers.h | 2 + src/xrpld/rpc/handlers/Simulate.cpp | 166 +++++++++++++++++++++++++ src/xrpld/rpc/handlers/Submit.cpp | 2 +- 6 files changed, 191 insertions(+), 11 deletions(-) create mode 100644 src/xrpld/rpc/handlers/Simulate.cpp diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index 42e9f0677ab..4a908356bb9 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -100,19 +100,22 @@ preflight1(PreflightContext const& ctx) } // No point in going any further if the transaction fee is malformed. - auto const fee = ctx.tx.getFieldAmount(sfFee); - if (!fee.native() || fee.negative() || !isLegalAmount(fee.xrp())) + if (!(ctx.flags & tapDRY_RUN)) { - JLOG(ctx.j.debug()) << "preflight1: invalid fee"; - return temBAD_FEE; - } + auto const fee = ctx.tx.getFieldAmount(sfFee); + if (!fee.native() || fee.negative() || !isLegalAmount(fee.xrp())) + { + JLOG(ctx.j.debug()) << "preflight1: invalid fee"; + return temBAD_FEE; + } - auto const spk = ctx.tx.getSigningPubKey(); + auto const spk = ctx.tx.getSigningPubKey(); - if (!spk.empty() && !publicKeyType(makeSlice(spk))) - { - JLOG(ctx.j.debug()) << "preflight1: invalid signing key"; - return temBAD_SIGNATURE; + if (!spk.empty() && !publicKeyType(makeSlice(spk))) + { + JLOG(ctx.j.debug()) << "preflight1: invalid signing key"; + return temBAD_SIGNATURE; + } } // An AccountTxnID field constrains transaction ordering more than the @@ -132,6 +135,8 @@ preflight1(PreflightContext const& ctx) NotTEC preflight2(PreflightContext const& ctx) { + if (ctx.flags & tapDRY_RUN) + return tesSUCCESS; auto const sigValid = checkValidity( ctx.app.getHashRouter(), ctx.tx, ctx.rules, ctx.app.config()); if (sigValid.first == Validity::SigBad) @@ -480,6 +485,8 @@ Transactor::apply() NotTEC Transactor::checkSign(PreclaimContext const& ctx) { + if (ctx.flags & tapDRY_RUN) + return tesSUCCESS; // If the pk is empty, then we must be multi-signing. if (ctx.tx.getSigningPubKey().empty()) return checkMultiSign(ctx); diff --git a/src/xrpld/ledger/ApplyView.h b/src/xrpld/ledger/ApplyView.h index f0166cd0b38..3b1cb69e4db 100644 --- a/src/xrpld/ledger/ApplyView.h +++ b/src/xrpld/ledger/ApplyView.h @@ -39,6 +39,10 @@ enum ApplyFlags : std::uint32_t { // Transaction came from a privileged source tapUNLIMITED = 0x400, + + // Transaction shouldn't be applied + // Signatures shouldn't be checked + tapDRY_RUN = 0x800 }; constexpr ApplyFlags diff --git a/src/xrpld/rpc/detail/Handler.cpp b/src/xrpld/rpc/detail/Handler.cpp index 4bac4610229..552e6b333a6 100644 --- a/src/xrpld/rpc/detail/Handler.cpp +++ b/src/xrpld/rpc/detail/Handler.cpp @@ -171,6 +171,7 @@ Handler const handlerArray[]{ {"server_state", byRef(&doServerState), Role::USER, NO_CONDITION}, {"sign", byRef(&doSign), Role::USER, NO_CONDITION}, {"sign_for", byRef(&doSignFor), Role::USER, NO_CONDITION}, + {"simulate", byRef(&doSimulate), Role::USER, NEEDS_CURRENT_LEDGER}, {"stop", byRef(&doStop), Role::ADMIN, NO_CONDITION}, {"submit", byRef(&doSubmit), Role::USER, NEEDS_CURRENT_LEDGER}, {"submit_multisigned", diff --git a/src/xrpld/rpc/handlers/Handlers.h b/src/xrpld/rpc/handlers/Handlers.h index 917ad38a741..d26b3873646 100644 --- a/src/xrpld/rpc/handlers/Handlers.h +++ b/src/xrpld/rpc/handlers/Handlers.h @@ -139,6 +139,8 @@ doSign(RPC::JsonContext&); Json::Value doSignFor(RPC::JsonContext&); Json::Value +doSimulate(RPC::JsonContext&); +Json::Value doCrawlShards(RPC::JsonContext&); Json::Value doStop(RPC::JsonContext&); diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp new file mode 100644 index 00000000000..c47b47d257d --- /dev/null +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -0,0 +1,166 @@ +//------------------------------------------------------------------------------ +/* + This file is part of rippled: https://github.com/ripple/rippled + Copyright (c) 2024 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 +#include +#include +#include +#include +#include +#include + +namespace ripple { + +class OpenLedger; + +// static NetworkOPs::FailHard +// getFailHard(RPC::JsonContext const& context) +// { +// return NetworkOPs::doFailHard( +// context.params.isMember("fail_hard") && +// context.params["fail_hard"].asBool()); +// } + +// { +// tx_blob: +// } +Json::Value +doSimulate(RPC::JsonContext& context) +{ + context.loadType = Resource::feeMediumBurdenRPC; + + // TODO: also support tx_json, and fee/sequence autofill + if (!context.params.isMember(jss::tx_blob)) + return rpcError(rpcINVALID_PARAMS); + + Json::Value jvResult; + + auto ret = strUnHex(context.params[jss::tx_blob].asString()); + + if (!ret || !ret->size()) + return rpcError(rpcINVALID_PARAMS); + + SerialIter sitTrans(makeSlice(*ret)); + + std::shared_ptr stpTrans; + + try + { + stpTrans = std::make_shared(std::ref(sitTrans)); + } + catch (std::exception& e) + { + jvResult[jss::error] = "invalidTransaction"; + jvResult[jss::error_exception] = e.what(); + + return jvResult; + } + + std::string reason; + auto tpTrans = std::make_shared(stpTrans, reason, context.app); + + try + { + // we check before adding to the batch + ApplyFlags flags = tapDRY_RUN; + + // if (getFailHard(context) == NetworkOps::FailHard::yes) + // flags |= tapFAIL_HARD; + + auto view = *context.app.openLedger().current(); + auto const result = context.app.getTxQ().apply( + context.app, view, tpTrans->getSTransaction(), flags, context.j); + // e.result = result.first; + // e.applied = result.second; + + std::string sToken; + std::string sHuman; + + transResultInfo(result.first, sToken, sHuman); + + jvResult[jss::engine_result] = sToken; + jvResult[jss::engine_result_code] = result.first; + jvResult[jss::engine_result_message] = sHuman; + jvResult[jss::applied] = result.second; + } + catch (std::exception& e) + { + jvResult[jss::error] = "internalSimulate"; + jvResult[jss::error_exception] = e.what(); + + return jvResult; + } + + try + { + jvResult[jss::tx_json] = tpTrans->getJson(JsonOptions::none); + jvResult[jss::tx_blob] = + strHex(tpTrans->getSTransaction()->getSerializer().peekData()); + + // if (temUNCERTAIN != tpTrans->getResult()) + // { + std::string sToken; + std::string sHuman; + + transResultInfo(tpTrans->getResult(), sToken, sHuman); + + // jvResult[jss::engine_result] = sToken; + // jvResult[jss::engine_result_code] = tpTrans->getResult(); + // jvResult[jss::engine_result_message] = sHuman; + + auto const submitResult = tpTrans->getSubmitResult(); + + jvResult[jss::accepted] = submitResult.any(); + // jvResult[jss::applied] = submitResult.applied; + jvResult[jss::broadcast] = submitResult.broadcast; + jvResult[jss::queued] = submitResult.queued; + jvResult[jss::kept] = submitResult.kept; + + if (auto currentLedgerState = tpTrans->getCurrentLedgerState()) + { + jvResult[jss::account_sequence_next] = safe_cast( + currentLedgerState->accountSeqNext); + jvResult[jss::account_sequence_available] = + safe_cast( + currentLedgerState->accountSeqAvail); + jvResult[jss::open_ledger_cost] = + to_string(currentLedgerState->minFeeRequired); + jvResult[jss::validated_ledger_index] = + safe_cast( + currentLedgerState->validatedLedger); + } + // } + + return jvResult; + } + catch (std::exception& e) + { + jvResult[jss::error] = "internalJson"; + jvResult[jss::error_exception] = e.what(); + + return jvResult; + } +} + +} // namespace ripple diff --git a/src/xrpld/rpc/handlers/Submit.cpp b/src/xrpld/rpc/handlers/Submit.cpp index 73fdc3822c2..90f7d456f32 100644 --- a/src/xrpld/rpc/handlers/Submit.cpp +++ b/src/xrpld/rpc/handlers/Submit.cpp @@ -40,7 +40,7 @@ getFailHard(RPC::JsonContext const& context) } // { -// tx_json: , +// tx_blob: , // secret: // } Json::Value From d151a20cc300284e34e4821d007da8be96cedfac Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 28 May 2024 12:07:19 -0400 Subject: [PATCH 02/42] fix print statement --- src/xrpld/app/tx/detail/Transactor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index 4a908356bb9..3463a381a91 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -1024,7 +1024,7 @@ Transactor::operator()() ctx_.apply(result); } - JLOG(j_.trace()) << (applied ? "applied" : "not applied") + JLOG(j_.trace()) << (applied ? "applied " : "not applied ") << transToken(result); return {result, applied}; From 4ab69aa255d469b53e8a41c6e7218ac46711e8b4 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 9 Jul 2024 11:08:21 -0400 Subject: [PATCH 03/42] get metadata to work --- src/test/app/TxQ_test.cpp | 7 +- src/xrpld/app/misc/TxQ.h | 9 +-- src/xrpld/app/misc/detail/TxQ.cpp | 14 ++-- src/xrpld/app/tx/apply.h | 3 +- src/xrpld/app/tx/applySteps.h | 3 +- src/xrpld/app/tx/detail/ApplyContext.cpp | 4 +- src/xrpld/app/tx/detail/ApplyContext.h | 2 +- src/xrpld/app/tx/detail/Transactor.cpp | 10 ++- src/xrpld/app/tx/detail/Transactor.h | 9 ++- src/xrpld/app/tx/detail/apply.cpp | 2 +- src/xrpld/app/tx/detail/applySteps.cpp | 4 +- src/xrpld/ledger/ApplyViewImpl.h | 9 ++- src/xrpld/ledger/detail/ApplyStateTable.cpp | 25 +++++-- src/xrpld/ledger/detail/ApplyStateTable.h | 3 +- src/xrpld/ledger/detail/ApplyViewImpl.cpp | 11 +++- src/xrpld/rpc/handlers/Simulate.cpp | 73 +++++++-------------- 16 files changed, 97 insertions(+), 91 deletions(-) diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index e7b70203c91..45926a37606 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -1062,11 +1062,10 @@ class TxQPosNegFlows_test : public beast::unit_test::suite env.app().openLedger().modify( [&](OpenView& view, beast::Journal j) { - // No need to initialize, since it's about to get set - bool didApply; - std::tie(parsed.ter, didApply) = ripple::apply( + auto result = ripple::apply( env.app(), view, *jt.stx, tapNONE, env.journal); - return didApply; + parsed.ter = result.first; + return result.second; }); env.postconditions(jt, parsed); } diff --git a/src/xrpld/app/misc/TxQ.h b/src/xrpld/app/misc/TxQ.h index b962d96d50f..e7c2983238f 100644 --- a/src/xrpld/app/misc/TxQ.h +++ b/src/xrpld/app/misc/TxQ.h @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -267,7 +268,7 @@ class TxQ the open ledger. If the transaction is queued, will return `{ terQUEUED, false }`. */ - std::pair + TxApplyResult apply( Application& app, OpenView& view, @@ -596,7 +597,7 @@ class TxQ PreflightResult const& pfresult); /// Attempt to apply the queued transaction to the open ledger. - std::pair + TxApplyResult apply(Application& app, OpenView& view, beast::Journal j); /// Potential @ref TxConsequences of applying this transaction @@ -730,7 +731,7 @@ class TxQ // Helper function for TxQ::apply. If a transaction's fee is high enough, // attempt to directly apply that transaction to the ledger. - std::optional> + std::optional tryDirectApply( Application& app, OpenView& view, @@ -837,7 +838,7 @@ class TxQ `accountIter` up to and including `tx`. Transactions following `tx` are not cleared. */ - std::pair + TxApplyResult tryClearAccountQueueUpThruTx( Application& app, OpenView& view, diff --git a/src/xrpld/app/misc/detail/TxQ.cpp b/src/xrpld/app/misc/detail/TxQ.cpp index 159a700cc3f..af071b6d2a0 100644 --- a/src/xrpld/app/misc/detail/TxQ.cpp +++ b/src/xrpld/app/misc/detail/TxQ.cpp @@ -288,7 +288,7 @@ TxQ::MaybeTx::MaybeTx( { } -std::pair +TxApplyResult TxQ::MaybeTx::apply(Application& app, OpenView& view, beast::Journal j) { // If the rules or flags change, preflight again @@ -500,7 +500,7 @@ TxQ::erase( return txQAccount.transactions.erase(begin, end); } -std::pair +TxApplyResult TxQ::tryClearAccountQueueUpThruTx( Application& app, OpenView& view, @@ -710,7 +710,7 @@ TxQ::tryClearAccountQueueUpThruTx( // b. The entire queue also has a (dynamic) maximum size. Transactions // beyond that limit are rejected. // -std::pair +TxApplyResult TxQ::apply( Application& app, OpenView& view, @@ -1456,7 +1456,7 @@ TxQ::accept(Application& app, OpenView& view) JLOG(j_.trace()) << "Applying queued transaction " << candidateIter->txID << " to open ledger."; - auto const [txnResult, didApply] = + auto const [txnResult, didApply, _metadata] = candidateIter->apply(app, view, j_); if (didApply) @@ -1653,7 +1653,7 @@ TxQ::getRequiredFeeLevel( return FeeMetrics::scaleFeeLevel(metricsSnapshot, view); } -std::optional> +std::optional TxQ::tryDirectApply( Application& app, OpenView& view, @@ -1693,7 +1693,7 @@ TxQ::tryDirectApply( JLOG(j_.trace()) << "Applying transaction " << transactionID << " to open ledger."; - auto const [txnResult, didApply] = + auto const [txnResult, didApply, metadata] = ripple::apply(app, view, *tx, flags, j); JLOG(j_.trace()) << "New transaction " << transactionID @@ -1719,7 +1719,7 @@ TxQ::tryDirectApply( } } } - return {std::pair(txnResult, didApply)}; + return TxApplyResult{txnResult, didApply, metadata}; } return {}; } diff --git a/src/xrpld/app/tx/apply.h b/src/xrpld/app/tx/apply.h index 6a0401e2b55..d7e95c9ecbb 100644 --- a/src/xrpld/app/tx/apply.h +++ b/src/xrpld/app/tx/apply.h @@ -25,6 +25,7 @@ #include #include #include +#include #include #include @@ -119,7 +120,7 @@ forceValidity(HashRouter& router, uint256 const& txid, Validity validity); @return A pair with the `TER` and a `bool` indicating whether or not the transaction was applied. */ -std::pair +TxApplyResult apply( Application& app, OpenView& view, diff --git a/src/xrpld/app/tx/applySteps.h b/src/xrpld/app/tx/applySteps.h index 1df537515e9..8d788e94bf7 100644 --- a/src/xrpld/app/tx/applySteps.h +++ b/src/xrpld/app/tx/applySteps.h @@ -28,6 +28,7 @@ namespace ripple { class Application; class STTx; class TxQ; +struct TxApplyResult; /** Return true if the transaction can claim a fee (tec), and the `ApplyFlags` do not allow soft failures. @@ -333,7 +334,7 @@ calculateDefaultBaseFee(ReadView const& view, STTx const& tx); @return A pair with the `TER` and a `bool` indicating whether or not the transaction was applied. */ -std::pair +TxApplyResult doApply(PreclaimResult const& preclaimResult, Application& app, OpenView& view); } // namespace ripple diff --git a/src/xrpld/app/tx/detail/ApplyContext.cpp b/src/xrpld/app/tx/detail/ApplyContext.cpp index 969af7960eb..5a0cc1eeeb2 100644 --- a/src/xrpld/app/tx/detail/ApplyContext.cpp +++ b/src/xrpld/app/tx/detail/ApplyContext.cpp @@ -53,10 +53,10 @@ ApplyContext::discard() view_.emplace(&base_, flags_); } -void +std::optional ApplyContext::apply(TER ter) { - view_->apply(base_, tx, ter, journal); + return view_->apply(base_, tx, ter, flags_ & tapDRY_RUN, journal); } std::size_t diff --git a/src/xrpld/app/tx/detail/ApplyContext.h b/src/xrpld/app/tx/detail/ApplyContext.h index 45de05a73db..57611e3a10f 100644 --- a/src/xrpld/app/tx/detail/ApplyContext.h +++ b/src/xrpld/app/tx/detail/ApplyContext.h @@ -81,7 +81,7 @@ class ApplyContext discard(); /** Apply the transaction result to the base. */ - void apply(TER); + std::optional apply(TER); /** Get the number of unapplied changes. */ std::size_t diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index 3463a381a91..986c114aa1c 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -486,7 +486,10 @@ NotTEC Transactor::checkSign(PreclaimContext const& ctx) { if (ctx.flags & tapDRY_RUN) + { + // TODO: ensure there is no signature included return tesSUCCESS; + } // If the pk is empty, then we must be multi-signing. if (ctx.tx.getSigningPubKey().empty()) return checkMultiSign(ctx); @@ -833,7 +836,7 @@ Transactor::reset(XRPAmount fee) } //------------------------------------------------------------------------------ -std::pair +TxApplyResult Transactor::operator()() { JLOG(j_.trace()) << "apply: " << ctx_.tx.getTransactionID(); @@ -1002,6 +1005,7 @@ Transactor::operator()() applied = false; } + std::optional metadata; if (applied) { // Transaction succeeded fully or (retries are not allowed and the @@ -1021,13 +1025,13 @@ Transactor::operator()() ctx_.destroyXRP(fee); // Once we call apply, we will no longer be able to look at view() - ctx_.apply(result); + metadata = ctx_.apply(result); } JLOG(j_.trace()) << (applied ? "applied " : "not applied ") << transToken(result); - return {result, applied}; + return {result, applied, metadata}; } } // namespace ripple diff --git a/src/xrpld/app/tx/detail/Transactor.h b/src/xrpld/app/tx/detail/Transactor.h index 27f22a0eb2e..940027b2981 100644 --- a/src/xrpld/app/tx/detail/Transactor.h +++ b/src/xrpld/app/tx/detail/Transactor.h @@ -82,6 +82,13 @@ struct PreclaimContext class TxConsequences; struct PreflightResult; +struct TxApplyResult +{ + TER first; + bool second; + std::optional metadata = std::nullopt; +}; + class Transactor { protected: @@ -100,7 +107,7 @@ class Transactor public: enum ConsequencesFactoryType { Normal, Blocker, Custom }; /** Process the transaction. */ - std::pair + TxApplyResult operator()(); ApplyView& diff --git a/src/xrpld/app/tx/detail/apply.cpp b/src/xrpld/app/tx/detail/apply.cpp index 103ec041074..c7e10224d85 100644 --- a/src/xrpld/app/tx/detail/apply.cpp +++ b/src/xrpld/app/tx/detail/apply.cpp @@ -105,7 +105,7 @@ forceValidity(HashRouter& router, uint256 const& txid, Validity validity) router.setFlags(txid, flags); } -std::pair +TxApplyResult apply( Application& app, OpenView& view, diff --git a/src/xrpld/app/tx/detail/applySteps.cpp b/src/xrpld/app/tx/detail/applySteps.cpp index 9ddaa3051c4..182143b6db3 100644 --- a/src/xrpld/app/tx/detail/applySteps.cpp +++ b/src/xrpld/app/tx/detail/applySteps.cpp @@ -337,7 +337,7 @@ TxConsequences::TxConsequences(STTx const& tx, std::uint32_t sequencesConsumed) sequencesConsumed_ = sequencesConsumed; } -static std::pair +static TxApplyResult invoke_apply(ApplyContext& ctx) { try @@ -435,7 +435,7 @@ calculateDefaultBaseFee(ReadView const& view, STTx const& tx) return Transactor::calculateBaseFee(view, tx); } -std::pair +TxApplyResult doApply(PreclaimResult const& preclaimResult, Application& app, OpenView& view) { if (preclaimResult.view.seq() != view.seq()) diff --git a/src/xrpld/ledger/ApplyViewImpl.h b/src/xrpld/ledger/ApplyViewImpl.h index 65678c5a6c4..74ee2908295 100644 --- a/src/xrpld/ledger/ApplyViewImpl.h +++ b/src/xrpld/ledger/ApplyViewImpl.h @@ -52,8 +52,13 @@ class ApplyViewImpl final : public detail::ApplyViewBase operation on this object is to call the destructor. */ - void - apply(OpenView& to, STTx const& tx, TER ter, beast::Journal j); + std::optional + apply( + OpenView& to, + STTx const& tx, + TER ter, + bool isDryRun, + beast::Journal j); /** Set the amount of currency delivered. diff --git a/src/xrpld/ledger/detail/ApplyStateTable.cpp b/src/xrpld/ledger/detail/ApplyStateTable.cpp index b849365c83f..fb6a7e314f0 100644 --- a/src/xrpld/ledger/detail/ApplyStateTable.cpp +++ b/src/xrpld/ledger/detail/ApplyStateTable.cpp @@ -109,19 +109,21 @@ ApplyStateTable::visit( } } -void +std::optional ApplyStateTable::apply( OpenView& to, STTx const& tx, TER ter, std::optional const& deliver, + bool isDryRun, beast::Journal j) { // Build metadata and insert auto const sTx = std::make_shared(); tx.add(*sTx); std::shared_ptr sMeta; - if (!to.open()) + std::optional metadata; + if (!to.open() || isDryRun) { TxMeta meta(tx.getTransactionID(), to.seq()); if (deliver) @@ -249,9 +251,12 @@ ApplyStateTable::apply( } } - // add any new modified nodes to the modification set - for (auto& mod : newMod) - to.rawReplace(mod.second); + if (!isDryRun) + { + // add any new modified nodes to the modification set + for (auto& mod : newMod) + to.rawReplace(mod.second); + } sMeta = std::make_shared(); meta.addRaw(*sMeta, ter, to.txCount()); @@ -259,9 +264,15 @@ ApplyStateTable::apply( // VFALCO For diagnostics do we want to show // metadata even when the base view is open? JLOG(j.trace()) << "metadata " << meta.getJson(JsonOptions::none); + + metadata = meta; + } + if (!isDryRun) + { + to.rawTxInsert(tx.getTransactionID(), sTx, sMeta); + apply(to); } - to.rawTxInsert(tx.getTransactionID(), sTx, sMeta); - apply(to); + return metadata; } //--- diff --git a/src/xrpld/ledger/detail/ApplyStateTable.h b/src/xrpld/ledger/detail/ApplyStateTable.h index d1616d095e5..c029b3386cb 100644 --- a/src/xrpld/ledger/detail/ApplyStateTable.h +++ b/src/xrpld/ledger/detail/ApplyStateTable.h @@ -64,12 +64,13 @@ class ApplyStateTable void apply(RawView& to) const; - void + std::optional apply( OpenView& to, STTx const& tx, TER ter, std::optional const& deliver, + bool isDryRun, beast::Journal j); bool diff --git a/src/xrpld/ledger/detail/ApplyViewImpl.cpp b/src/xrpld/ledger/detail/ApplyViewImpl.cpp index ffbf681cd20..1aea83acdac 100644 --- a/src/xrpld/ledger/detail/ApplyViewImpl.cpp +++ b/src/xrpld/ledger/detail/ApplyViewImpl.cpp @@ -28,10 +28,15 @@ ApplyViewImpl::ApplyViewImpl(ReadView const* base, ApplyFlags flags) { } -void -ApplyViewImpl::apply(OpenView& to, STTx const& tx, TER ter, beast::Journal j) +std::optional +ApplyViewImpl::apply( + OpenView& to, + STTx const& tx, + TER ter, + bool isDryRun, + beast::Journal j) { - items_.apply(to, tx, ter, deliver_, j); + return items_.apply(to, tx, ter, deliver_, isDryRun, j); } std::size_t diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index c47b47d257d..bf4be8e7da9 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -30,6 +30,23 @@ #include #include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + namespace ripple { class OpenLedger; @@ -88,7 +105,7 @@ doSimulate(RPC::JsonContext& context) // if (getFailHard(context) == NetworkOps::FailHard::yes) // flags |= tapFAIL_HARD; - auto view = *context.app.openLedger().current(); + OpenView view = *context.app.openLedger().current(); auto const result = context.app.getTxQ().apply( context.app, view, tpTrans->getSTransaction(), flags, context.j); // e.result = result.first; @@ -103,60 +120,14 @@ doSimulate(RPC::JsonContext& context) jvResult[jss::engine_result_code] = result.first; jvResult[jss::engine_result_message] = sHuman; jvResult[jss::applied] = result.second; - } - catch (std::exception& e) - { - jvResult[jss::error] = "internalSimulate"; - jvResult[jss::error_exception] = e.what(); - - return jvResult; - } - - try - { - jvResult[jss::tx_json] = tpTrans->getJson(JsonOptions::none); - jvResult[jss::tx_blob] = - strHex(tpTrans->getSTransaction()->getSerializer().peekData()); - - // if (temUNCERTAIN != tpTrans->getResult()) - // { - std::string sToken; - std::string sHuman; - - transResultInfo(tpTrans->getResult(), sToken, sHuman); - - // jvResult[jss::engine_result] = sToken; - // jvResult[jss::engine_result_code] = tpTrans->getResult(); - // jvResult[jss::engine_result_message] = sHuman; - - auto const submitResult = tpTrans->getSubmitResult(); - - jvResult[jss::accepted] = submitResult.any(); - // jvResult[jss::applied] = submitResult.applied; - jvResult[jss::broadcast] = submitResult.broadcast; - jvResult[jss::queued] = submitResult.queued; - jvResult[jss::kept] = submitResult.kept; - - if (auto currentLedgerState = tpTrans->getCurrentLedgerState()) - { - jvResult[jss::account_sequence_next] = safe_cast( - currentLedgerState->accountSeqNext); - jvResult[jss::account_sequence_available] = - safe_cast( - currentLedgerState->accountSeqAvail); - jvResult[jss::open_ledger_cost] = - to_string(currentLedgerState->minFeeRequired); - jvResult[jss::validated_ledger_index] = - safe_cast( - currentLedgerState->validatedLedger); - } - // } - + if (result.metadata) + jvResult[jss::metadata] = + result.metadata->getJson(JsonOptions::none); return jvResult; } catch (std::exception& e) { - jvResult[jss::error] = "internalJson"; + jvResult[jss::error] = "internalSimulate"; jvResult[jss::error_exception] = e.what(); return jvResult; From 360ed6a540207b69fd8834d163e3ba31f0ffc345 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 9 Jul 2024 13:51:13 -0400 Subject: [PATCH 04/42] clean up --- src/xrpld/app/tx/apply.h | 2 +- src/xrpld/app/tx/applySteps.h | 8 ++++- src/xrpld/app/tx/detail/ApplyContext.h | 6 ++++ src/xrpld/app/tx/detail/Transactor.cpp | 5 +++ src/xrpld/app/tx/detail/Transactor.h | 7 ---- src/xrpld/rpc/handlers/Simulate.cpp | 44 ++++++++------------------ 6 files changed, 32 insertions(+), 40 deletions(-) diff --git a/src/xrpld/app/tx/apply.h b/src/xrpld/app/tx/apply.h index d7e95c9ecbb..b00d47dec26 100644 --- a/src/xrpld/app/tx/apply.h +++ b/src/xrpld/app/tx/apply.h @@ -25,7 +25,7 @@ #include #include #include -#include +#include #include #include diff --git a/src/xrpld/app/tx/applySteps.h b/src/xrpld/app/tx/applySteps.h index 8d788e94bf7..59337725d1d 100644 --- a/src/xrpld/app/tx/applySteps.h +++ b/src/xrpld/app/tx/applySteps.h @@ -28,7 +28,13 @@ namespace ripple { class Application; class STTx; class TxQ; -struct TxApplyResult; + +struct TxApplyResult +{ + TER first; + bool second; + std::optional metadata = std::nullopt; +}; /** Return true if the transaction can claim a fee (tec), and the `ApplyFlags` do not allow soft failures. diff --git a/src/xrpld/app/tx/detail/ApplyContext.h b/src/xrpld/app/tx/detail/ApplyContext.h index 57611e3a10f..e0d8bee3041 100644 --- a/src/xrpld/app/tx/detail/ApplyContext.h +++ b/src/xrpld/app/tx/detail/ApplyContext.h @@ -69,6 +69,12 @@ class ApplyContext return *view_; } + ApplyFlags const& + flags() + { + return flags_; + } + /** Sets the DeliveredAmount field in the metadata */ void deliver(STAmount const& amount) diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index 986c114aa1c..455d9987967 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -1028,6 +1028,11 @@ Transactor::operator()() metadata = ctx_.apply(result); } + if (ctx_.flags() & tapDRY_RUN) + { + applied = false; + } + JLOG(j_.trace()) << (applied ? "applied " : "not applied ") << transToken(result); diff --git a/src/xrpld/app/tx/detail/Transactor.h b/src/xrpld/app/tx/detail/Transactor.h index 940027b2981..85e9c1f42cf 100644 --- a/src/xrpld/app/tx/detail/Transactor.h +++ b/src/xrpld/app/tx/detail/Transactor.h @@ -82,13 +82,6 @@ struct PreclaimContext class TxConsequences; struct PreflightResult; -struct TxApplyResult -{ - TER first; - bool second; - std::optional metadata = std::nullopt; -}; - class Transactor { protected: diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index bf4be8e7da9..136d5ba26b0 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -30,35 +30,8 @@ #include #include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - namespace ripple { -class OpenLedger; - -// static NetworkOPs::FailHard -// getFailHard(RPC::JsonContext const& context) -// { -// return NetworkOPs::doFailHard( -// context.params.isMember("fail_hard") && -// context.params["fail_hard"].asBool()); -// } - // { // tx_blob: // } @@ -105,24 +78,33 @@ doSimulate(RPC::JsonContext& context) // if (getFailHard(context) == NetworkOps::FailHard::yes) // flags |= tapFAIL_HARD; + // Process the transaction OpenView view = *context.app.openLedger().current(); auto const result = context.app.getTxQ().apply( context.app, view, tpTrans->getSTransaction(), flags, context.j); - // e.result = result.first; - // e.applied = result.second; + jvResult[jss::applied] = result.second; + + // Convert the TER to human-readable values std::string sToken; std::string sHuman; - transResultInfo(result.first, sToken, sHuman); + // Engine result jvResult[jss::engine_result] = sToken; jvResult[jss::engine_result_code] = result.first; jvResult[jss::engine_result_message] = sHuman; - jvResult[jss::applied] = result.second; + if (sToken == "tesSUCCESS") + { + static const std::string alternateSuccessMessage = + "The simulated transaction would have been applied."; + jvResult[jss::engine_result_message] = alternateSuccessMessage; + } + if (result.metadata) jvResult[jss::metadata] = result.metadata->getJson(JsonOptions::none); + return jvResult; } catch (std::exception& e) From 628e1e3b7eadfff488c0d0a76e975c3513c3e1ae Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 9 Jul 2024 16:18:50 -0400 Subject: [PATCH 05/42] add basic tx_json support --- src/xrpld/rpc/handlers/Simulate.cpp | 67 ++++++++++++++++++++++------- 1 file changed, 51 insertions(+), 16 deletions(-) diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index 136d5ba26b0..58e613abf99 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -29,6 +29,7 @@ #include #include #include +#include namespace ripple { @@ -40,31 +41,65 @@ doSimulate(RPC::JsonContext& context) { context.loadType = Resource::feeMediumBurdenRPC; - // TODO: also support tx_json, and fee/sequence autofill - if (!context.params.isMember(jss::tx_blob)) - return rpcError(rpcINVALID_PARAMS); - + std::shared_ptr stpTrans; Json::Value jvResult; - auto ret = strUnHex(context.params[jss::tx_blob].asString()); + // TODO: also support fee/sequence autofill + if (context.params.isMember(jss::tx_blob)) + { + if (context.params.isMember(jss::tx_json)) + { + return rpcError(rpcINVALID_PARAMS); + } - if (!ret || !ret->size()) - return rpcError(rpcINVALID_PARAMS); + auto ret = strUnHex(context.params[jss::tx_blob].asString()); - SerialIter sitTrans(makeSlice(*ret)); + if (!ret || !ret->size()) + return rpcError(rpcINVALID_PARAMS); - std::shared_ptr stpTrans; + SerialIter sitTrans(makeSlice(*ret)); - try - { - stpTrans = std::make_shared(std::ref(sitTrans)); + try + { + stpTrans = std::make_shared(std::ref(sitTrans)); + } + catch (std::exception& e) + { + jvResult[jss::error] = "invalidTransaction"; + jvResult[jss::error_exception] = e.what(); + + return jvResult; + } } - catch (std::exception& e) + else { - jvResult[jss::error] = "invalidTransaction"; - jvResult[jss::error_exception] = e.what(); + if (!context.params.isMember(jss::tx_json)) + { + return rpcError(rpcINVALID_PARAMS); + } - return jvResult; + Json::Value& tx_json(context.params[jss::tx_json]); + + STParsedJSONObject parsed(std::string(jss::tx_json), tx_json); + if (!parsed.object.has_value()) + { + jvResult[jss::error] = parsed.error[jss::error]; + jvResult[jss::error_code] = parsed.error[jss::error_code]; + jvResult[jss::error_message] = parsed.error[jss::error_message]; + return jvResult; + } + + try + { + stpTrans = std::make_shared(std::move(parsed.object.value())); + } + catch (std::exception& e) + { + jvResult[jss::error] = "invalidTransaction"; + jvResult[jss::error_exception] = e.what(); + + return jvResult; + } } std::string reason; From 0149c65c33761f6a118e4607b415a130162b5324 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 10 Jul 2024 12:53:54 -0400 Subject: [PATCH 06/42] basic tests --- src/test/rpc/Simulate_test.cpp | 144 +++++++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+) create mode 100644 src/test/rpc/Simulate_test.cpp diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp new file mode 100644 index 00000000000..1a597b8536c --- /dev/null +++ b/src/test/rpc/Simulate_test.cpp @@ -0,0 +1,144 @@ +//------------------------------------------------------------------------------ +/* + 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 +#include +#include +#include + +#include +#include + +namespace ripple { + +namespace test { + +class Simulate_test : public beast::unit_test::suite +{ + void + checkBasicReturnValidity(Json::Value result) + { + BEAST_EXPECT(result[jss::applied] == false); + BEAST_EXPECT(result.isMember(jss::engine_result)); + BEAST_EXPECT(result.isMember(jss::engine_result_code)); + BEAST_EXPECT(result.isMember(jss::engine_result_message)); + } + + void + testParamErrors() + { + testcase("Test errors in the parameters"); + + using namespace jtx; + Env env(*this); + + { + auto resp = env.rpc("json", "simulate"); + BEAST_EXPECT(resp[jss::error_message] == "Syntax error."); + } + + { + Json::Value params; + params[jss::tx_json] = Json::objectValue; + params[jss::tx_blob] = "1200"; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == "Invalid parameters."); + } + } + void + testBasic() + { + testcase("Basic test"); + + using namespace jtx; + Env env(*this); + + { + Json::Value tx; + + auto newDomain = "123ABC"; + + tx[jss::Account] = Account::master.human(); + tx[jss::TransactionType] = jss::AccountSet; + tx[sfDomain.jsonName] = newDomain; + tx[sfSigningPubKey.jsonName] = ""; + tx[sfTxnSignature.jsonName] = ""; + tx[sfSequence.jsonName] = 1; + tx[sfFee.jsonName] = "12"; + + Json::Value params; + params[jss::tx_json] = tx; + + auto resp = env.rpc("json", "simulate", to_string(params)); + auto result = resp[jss::result]; + checkBasicReturnValidity(result); + + BEAST_EXPECT(result[jss::engine_result] == "tesSUCCESS"); + BEAST_EXPECT(result[jss::engine_result_code] == 0); + BEAST_EXPECT( + result[jss::engine_result_message] == + "The simulated transaction would have been applied."); + + if (BEAST_EXPECT(result.isMember(jss::metadata))) + { + auto metadata = result[jss::metadata]; + if (BEAST_EXPECT(metadata.isMember(sfAffectedNodes.jsonName))) + { + auto node = metadata[sfAffectedNodes.jsonName][0u]; + if (BEAST_EXPECT(node.isMember(sfModifiedNode.jsonName))) + { + auto modifiedNode = node[sfModifiedNode.jsonName]; + BEAST_EXPECT( + modifiedNode[sfLedgerEntryType.jsonName] == + "AccountRoot"); + auto finalFields = modifiedNode[sfFinalFields.jsonName]; + BEAST_EXPECT( + finalFields[sfDomain.jsonName] == newDomain); + } + } + BEAST_EXPECT(metadata[sfTransactionIndex.jsonName] == 0); + BEAST_EXPECT( + metadata[sfTransactionResult.jsonName] == "tesSUCCESS"); + } + + // TODO: check that the ledger wasn't affected + } + } + +public: + void + run() override + { + testParamErrors(); + testBasic(); + } +}; + +BEAST_DEFINE_TESTSUITE(Simulate, rpc, ripple); + +} // namespace test + +} // namespace ripple From f555b73ad536c250577eed3cb8be33e38f3db8f9 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 10 Jul 2024 15:57:50 -0400 Subject: [PATCH 07/42] autofill fee --- src/test/rpc/Simulate_test.cpp | 9 ++++ src/xrpld/rpc/handlers/Simulate.cpp | 80 +++++++++++++++++++++++++++-- 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp index 1a597b8536c..722b50f7c40 100644 --- a/src/test/rpc/Simulate_test.cpp +++ b/src/test/rpc/Simulate_test.cpp @@ -67,6 +67,15 @@ class Simulate_test : public beast::unit_test::suite BEAST_EXPECT( resp[jss::result][jss::error_message] == "Invalid parameters."); } + + { + Json::Value params; + params[jss::tx_blob] = "1200"; + params[jss::binary] = "100"; + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == "Invalid parameters."); + } } void testBasic() diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index 58e613abf99..45c14959942 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -30,9 +30,50 @@ #include #include #include +#include namespace ripple { +Json::Value +getFee(Application& app, Role& role) +{ + XRPAmount const feeDefault = app.config().FEES.reference_fee; + + auto ledger = app.openLedger().current(); + // Administrative and identified endpoints are exempt from local fees. + XRPAmount const loadFee = scaleFeeLoad( + feeDefault, app.getFeeTrack(), ledger->fees(), isUnlimited(role)); + XRPAmount fee = loadFee; + { + auto const metrics = app.getTxQ().getMetrics(*ledger); + auto const baseFee = ledger->fees().base; + auto escalatedFee = + toDrops(metrics.openLedgerFeeLevel - FeeLevel64(1), baseFee) + 1; + fee = std::max(fee, escalatedFee); + } + + auto const limit = [&]() { + // Scale fee units to drops: + auto const result = mulDiv( + feeDefault, + RPC::Tuning::defaultAutoFillFeeMultiplier, + RPC::Tuning::defaultAutoFillFeeDivisor); + if (!result) + Throw("mulDiv"); + return *result; + }(); + + if (fee > limit) + { + std::stringstream ss; + ss << "Fee of " << fee << " exceeds the requested tx limit of " + << limit; + return RPC::make_error(rpcHIGH_FEE, ss.str()); + } + + return fee.jsonClipped(); +} + // { // tx_blob: // } @@ -44,6 +85,16 @@ doSimulate(RPC::JsonContext& context) std::shared_ptr stpTrans; Json::Value jvResult; + // check validity of `binary` param + if (context.params.isMember(jss::binary)) + { + auto const binary = context.params[jss::binary]; + if (!binary.isBool()) + { + return rpcError(rpcINVALID_PARAMS); + } + } + // TODO: also support fee/sequence autofill if (context.params.isMember(jss::tx_blob)) { @@ -70,6 +121,8 @@ doSimulate(RPC::JsonContext& context) return jvResult; } + + jvResult[jss::tx_blob] = context.params[jss::tx_blob]; } else { @@ -79,6 +132,10 @@ doSimulate(RPC::JsonContext& context) } Json::Value& tx_json(context.params[jss::tx_json]); + if (!tx_json.isMember(jss::Fee)) + { + tx_json[jss::Fee] = getFee(context.app, context.role); + } STParsedJSONObject parsed(std::string(jss::tx_json), tx_json); if (!parsed.object.has_value()) @@ -100,6 +157,8 @@ doSimulate(RPC::JsonContext& context) return jvResult; } + + jvResult[jss::tx_json] = tx_json; } std::string reason; @@ -110,9 +169,6 @@ doSimulate(RPC::JsonContext& context) // we check before adding to the batch ApplyFlags flags = tapDRY_RUN; - // if (getFailHard(context) == NetworkOps::FailHard::yes) - // flags |= tapFAIL_HARD; - // Process the transaction OpenView view = *context.app.openLedger().current(); auto const result = context.app.getTxQ().apply( @@ -120,6 +176,9 @@ doSimulate(RPC::JsonContext& context) jvResult[jss::applied] = result.second; + const bool isBinaryOutput = + context.params.get(jss::binary, false).asBool(); + // Convert the TER to human-readable values std::string sToken; std::string sHuman; @@ -137,8 +196,19 @@ doSimulate(RPC::JsonContext& context) } if (result.metadata) - jvResult[jss::metadata] = - result.metadata->getJson(JsonOptions::none); + { + if (isBinaryOutput) + { + auto const metaBlob = + result.metadata->getAsObject().getSerializer().getData(); + jvResult[jss::metadata] = strHex(makeSlice(metaBlob)); + } + else + { + jvResult[jss::metadata] = + result.metadata->getJson(JsonOptions::none); + } + } return jvResult; } From 51cf3c66f20de746d5f026ad42db86baf8b441bb Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 10 Jul 2024 16:13:51 -0400 Subject: [PATCH 08/42] autofill Sequence/SigningPubKey/TxnSignature --- src/xrpld/rpc/handlers/Simulate.cpp | 35 +++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index 45c14959942..aa1b8d45b29 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -136,6 +136,41 @@ doSimulate(RPC::JsonContext& context) { tx_json[jss::Fee] = getFee(context.app, context.role); } + if (!tx_json.isMember(sfSigningPubKey.jsonName)) + { + tx_json[sfSigningPubKey.jsonName] = ""; + } + if (!tx_json.isMember(sfTxnSignature.jsonName)) + { + tx_json[sfTxnSignature.jsonName] = ""; + } + if (!tx_json.isMember(jss::Sequence)) + { + bool const hasTicketSeq = + tx_json.isMember(sfTicketSequence.jsonName); + auto const srcAddressID = + *(parseBase58(tx_json[jss::Account].asString())); + if (!srcAddressID) + { + return RPC::make_error( + rpcSRC_ACT_MALFORMED, + RPC::invalid_field_message("tx_json.Account")); + } + std::shared_ptr sle = + context.app.openLedger().current()->read( + keylet::account(srcAddressID)); + if (!hasTicketSeq && !sle) + { + JLOG(context.app.journal("Simulate").debug()) + << "simulate: Failed to find source account " + << "in current ledger: " << toBase58(srcAddressID); + + return rpcError(rpcSRC_ACT_NOT_FOUND); + } + tx_json[jss::Sequence] = hasTicketSeq + ? 0 + : context.app.getTxQ().nextQueuableSeq(sle).value(); + } STParsedJSONObject parsed(std::string(jss::tx_json), tx_json); if (!parsed.object.has_value()) From 04e76d2d671b3e2e53f54c6c0555b052494759c8 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 10 Jul 2024 16:27:30 -0400 Subject: [PATCH 09/42] better erroring --- src/xrpld/rpc/handlers/Simulate.cpp | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index aa1b8d45b29..07e773469dc 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -106,7 +106,8 @@ doSimulate(RPC::JsonContext& context) auto ret = strUnHex(context.params[jss::tx_blob].asString()); if (!ret || !ret->size()) - return rpcError(rpcINVALID_PARAMS); + return RPC::make_error( + rpcINVALID_PARAMS, RPC::invalid_field_message("tx_blob")); SerialIter sitTrans(makeSlice(*ret)); @@ -121,8 +122,6 @@ doSimulate(RPC::JsonContext& context) return jvResult; } - - jvResult[jss::tx_blob] = context.params[jss::tx_blob]; } else { @@ -140,10 +139,22 @@ doSimulate(RPC::JsonContext& context) { tx_json[sfSigningPubKey.jsonName] = ""; } + else if (tx_json[sfSigningPubKey.jsonName] != "") + { + return RPC::make_error( + rpcINVALID_PARAMS, + RPC::invalid_field_message("tx_json.SigningPubKey")); + } if (!tx_json.isMember(sfTxnSignature.jsonName)) { tx_json[sfTxnSignature.jsonName] = ""; } + else if (tx_json[sfTxnSignature.jsonName] != "") + { + return RPC::make_error( + rpcINVALID_PARAMS, + RPC::invalid_field_message("tx_json.TxnSignature")); + } if (!tx_json.isMember(jss::Sequence)) { bool const hasTicketSeq = @@ -192,8 +203,6 @@ doSimulate(RPC::JsonContext& context) return jvResult; } - - jvResult[jss::tx_json] = tx_json; } std::string reason; @@ -244,6 +253,15 @@ doSimulate(RPC::JsonContext& context) result.metadata->getJson(JsonOptions::none); } } + if (isBinaryOutput) + { + auto const txBlob = stpTrans->getSerializer().getData(); + jvResult[jss::tx_blob] = strHex(makeSlice(txBlob)); + } + else + { + jvResult[jss::tx_json] = tpTrans->getJson(JsonOptions::none); + } return jvResult; } From eb691dcc4575588cc29f280052f649a0f82e2338 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 10 Jul 2024 17:51:18 -0400 Subject: [PATCH 10/42] run tx_blob through autofilling --- src/xrpld/rpc/detail/TransactionSign.cpp | 48 ++++- src/xrpld/rpc/detail/TransactionSign.h | 12 ++ src/xrpld/rpc/handlers/Simulate.cpp | 213 ++++++++++------------- 3 files changed, 148 insertions(+), 125 deletions(-) diff --git a/src/xrpld/rpc/detail/TransactionSign.cpp b/src/xrpld/rpc/detail/TransactionSign.cpp index 1fee84c683b..5f0295f9782 100644 --- a/src/xrpld/rpc/detail/TransactionSign.cpp +++ b/src/xrpld/rpc/detail/TransactionSign.cpp @@ -21,7 +21,6 @@ #include #include #include -#include #include #include #include @@ -29,7 +28,6 @@ #include #include #include -#include #include #include #include @@ -709,6 +707,50 @@ transactionFormatResultImpl(Transaction::pointer tpTrans, unsigned apiVersion) //------------------------------------------------------------------------------ +Json::Value +getCurrentFee( + Role const role, + Config const& config, + LoadFeeTrack const& feeTrack, + TxQ const& txQ, + Application const& app, + int mult, + int div) +{ + XRPAmount const feeDefault = config.FEES.reference_fee; + + auto ledger = app.openLedger().current(); + // Administrative and identified endpoints are exempt from local fees. + XRPAmount const loadFee = + scaleFeeLoad(feeDefault, feeTrack, ledger->fees(), isUnlimited(role)); + XRPAmount fee = loadFee; + { + auto const metrics = txQ.getMetrics(*ledger); + auto const baseFee = ledger->fees().base; + auto escalatedFee = + toDrops(metrics.openLedgerFeeLevel - FeeLevel64(1), baseFee) + 1; + fee = std::max(fee, escalatedFee); + } + + auto const limit = [&]() { + // Scale fee units to drops: + auto const result = mulDiv(feeDefault, mult, div); + if (!result) + Throw("mulDiv"); + return *result; + }(); + + if (fee > limit) + { + std::stringstream ss; + ss << "Fee of " << fee << " exceeds the requested tx limit of " + << limit; + return RPC::make_error(rpcHIGH_FEE, ss.str()); + } + + return fee.jsonClipped(); +} + Json::Value checkFee( Json::Value& request, @@ -798,7 +840,7 @@ checkFee( return RPC::make_error(rpcHIGH_FEE, ss.str()); } - tx[jss::Fee] = fee.jsonClipped(); + tx[jss::Fee] = getCurrentFee(role, config, feeTrack, txQ, app, mult, div); return Json::Value(); } diff --git a/src/xrpld/rpc/detail/TransactionSign.h b/src/xrpld/rpc/detail/TransactionSign.h index 36fb60555cf..59c200e58d4 100644 --- a/src/xrpld/rpc/detail/TransactionSign.h +++ b/src/xrpld/rpc/detail/TransactionSign.h @@ -23,6 +23,8 @@ #include #include #include +#include +#include namespace ripple { @@ -34,6 +36,16 @@ class TxQ; namespace RPC { +Json::Value +getCurrentFee( + Role const role, + Config const& config, + LoadFeeTrack const& feeTrack, + TxQ const& txQ, + Application const& app, + int mult = Tuning::defaultAutoFillFeeMultiplier, + int div = Tuning::defaultAutoFillFeeDivisor); + /** Fill in the fee on behalf of the client. This is called when the client does not explicitly specify the fee. The client may also put a ceiling on the amount of the fee. This ceiling diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index 07e773469dc..08aaadcba0f 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -30,60 +30,84 @@ #include #include #include -#include namespace ripple { -Json::Value -getFee(Application& app, Role& role) +std::optional +autofillTx(Json::Value& tx_json, RPC::JsonContext& context) { - XRPAmount const feeDefault = app.config().FEES.reference_fee; - - auto ledger = app.openLedger().current(); - // Administrative and identified endpoints are exempt from local fees. - XRPAmount const loadFee = scaleFeeLoad( - feeDefault, app.getFeeTrack(), ledger->fees(), isUnlimited(role)); - XRPAmount fee = loadFee; + if (!tx_json.isMember(jss::Fee)) { - auto const metrics = app.getTxQ().getMetrics(*ledger); - auto const baseFee = ledger->fees().base; - auto escalatedFee = - toDrops(metrics.openLedgerFeeLevel - FeeLevel64(1), baseFee) + 1; - fee = std::max(fee, escalatedFee); + tx_json[jss::Fee] = RPC::getCurrentFee( + context.role, + context.app.config(), + context.app.getFeeTrack(), + context.app.getTxQ(), + context.app); } - - auto const limit = [&]() { - // Scale fee units to drops: - auto const result = mulDiv( - feeDefault, - RPC::Tuning::defaultAutoFillFeeMultiplier, - RPC::Tuning::defaultAutoFillFeeDivisor); - if (!result) - Throw("mulDiv"); - return *result; - }(); - - if (fee > limit) + if (!tx_json.isMember(sfSigningPubKey.jsonName)) + { + tx_json[sfSigningPubKey.jsonName] = ""; + } + else if (tx_json[sfSigningPubKey.jsonName] != "") + { + return RPC::make_error( + rpcINVALID_PARAMS, + RPC::invalid_field_message("tx_json.SigningPubKey")); + } + if (!tx_json.isMember(sfTxnSignature.jsonName)) { - std::stringstream ss; - ss << "Fee of " << fee << " exceeds the requested tx limit of " - << limit; - return RPC::make_error(rpcHIGH_FEE, ss.str()); + tx_json[sfTxnSignature.jsonName] = ""; } + else if (tx_json[sfTxnSignature.jsonName] != "") + { + return RPC::make_error( + rpcINVALID_PARAMS, + RPC::invalid_field_message("tx_json.TxnSignature")); + } + if (!tx_json.isMember(jss::Sequence)) + { + bool const hasTicketSeq = tx_json.isMember(sfTicketSequence.jsonName); + auto const srcAddressID = + *(parseBase58(tx_json[jss::Account].asString())); + if (!srcAddressID) + { + return RPC::make_error( + rpcSRC_ACT_MALFORMED, + RPC::invalid_field_message("tx_json.Account")); + } + std::shared_ptr sle = + context.app.openLedger().current()->read( + keylet::account(srcAddressID)); + if (!hasTicketSeq && !sle) + { + JLOG(context.app.journal("Simulate").debug()) + << "simulate: Failed to find source account " + << "in current ledger: " << toBase58(srcAddressID); - return fee.jsonClipped(); + return rpcError(rpcSRC_ACT_NOT_FOUND); + } + tx_json[jss::Sequence] = hasTicketSeq + ? 0 + : context.app.getTxQ().nextQueuableSeq(sle).value(); + } + + return std::nullopt; } // { -// tx_blob: +// tx_blob: , +// tx_json: , +// binary: // } +// tx_blob XOR tx_json Json::Value doSimulate(RPC::JsonContext& context) { context.loadType = Resource::feeMediumBurdenRPC; - std::shared_ptr stpTrans; - Json::Value jvResult; + Json::Value jvResult; // the returned result + Json::Value tx_json; // the tx as a JSON // check validity of `binary` param if (context.params.isMember(jss::binary)) @@ -95,122 +119,67 @@ doSimulate(RPC::JsonContext& context) } } - // TODO: also support fee/sequence autofill if (context.params.isMember(jss::tx_blob)) { if (context.params.isMember(jss::tx_json)) { + // both `tx_blob` and `tx_json` included return rpcError(rpcINVALID_PARAMS); } - auto ret = strUnHex(context.params[jss::tx_blob].asString()); + auto unHexed = strUnHex(context.params[jss::tx_blob].asString()); - if (!ret || !ret->size()) + if (!unHexed || !unHexed->size()) return RPC::make_error( rpcINVALID_PARAMS, RPC::invalid_field_message("tx_blob")); - SerialIter sitTrans(makeSlice(*ret)); - - try - { - stpTrans = std::make_shared(std::ref(sitTrans)); - } - catch (std::exception& e) - { - jvResult[jss::error] = "invalidTransaction"; - jvResult[jss::error_exception] = e.what(); - - return jvResult; - } + SerialIter sitTrans(makeSlice(*unHexed)); + tx_json = + STObject(std::ref(sitTrans), sfGeneric).getJson(JsonOptions::none); } else { if (!context.params.isMember(jss::tx_json)) { + // neither `tx_blob` nor `tx_json` included` return rpcError(rpcINVALID_PARAMS); } - Json::Value& tx_json(context.params[jss::tx_json]); - if (!tx_json.isMember(jss::Fee)) - { - tx_json[jss::Fee] = getFee(context.app, context.role); - } - if (!tx_json.isMember(sfSigningPubKey.jsonName)) - { - tx_json[sfSigningPubKey.jsonName] = ""; - } - else if (tx_json[sfSigningPubKey.jsonName] != "") - { - return RPC::make_error( - rpcINVALID_PARAMS, - RPC::invalid_field_message("tx_json.SigningPubKey")); - } - if (!tx_json.isMember(sfTxnSignature.jsonName)) - { - tx_json[sfTxnSignature.jsonName] = ""; - } - else if (tx_json[sfTxnSignature.jsonName] != "") - { - return RPC::make_error( - rpcINVALID_PARAMS, - RPC::invalid_field_message("tx_json.TxnSignature")); - } - if (!tx_json.isMember(jss::Sequence)) - { - bool const hasTicketSeq = - tx_json.isMember(sfTicketSequence.jsonName); - auto const srcAddressID = - *(parseBase58(tx_json[jss::Account].asString())); - if (!srcAddressID) - { - return RPC::make_error( - rpcSRC_ACT_MALFORMED, - RPC::invalid_field_message("tx_json.Account")); - } - std::shared_ptr sle = - context.app.openLedger().current()->read( - keylet::account(srcAddressID)); - if (!hasTicketSeq && !sle) - { - JLOG(context.app.journal("Simulate").debug()) - << "simulate: Failed to find source account " - << "in current ledger: " << toBase58(srcAddressID); + tx_json = context.params[jss::tx_json]; + } - return rpcError(rpcSRC_ACT_NOT_FOUND); - } - tx_json[jss::Sequence] = hasTicketSeq - ? 0 - : context.app.getTxQ().nextQueuableSeq(sle).value(); - } + // autofill fields if they're not included (e.g. `Fee`, `Sequence`) + if (auto error = autofillTx(tx_json, context)) + return *error; - STParsedJSONObject parsed(std::string(jss::tx_json), tx_json); - if (!parsed.object.has_value()) - { - jvResult[jss::error] = parsed.error[jss::error]; - jvResult[jss::error_code] = parsed.error[jss::error_code]; - jvResult[jss::error_message] = parsed.error[jss::error_message]; - return jvResult; - } + STParsedJSONObject parsed(std::string(jss::tx_json), tx_json); + if (!parsed.object.has_value()) + { + jvResult[jss::error] = parsed.error[jss::error]; + jvResult[jss::error_code] = parsed.error[jss::error_code]; + jvResult[jss::error_message] = parsed.error[jss::error_message]; + return jvResult; + } - try - { - stpTrans = std::make_shared(std::move(parsed.object.value())); - } - catch (std::exception& e) - { - jvResult[jss::error] = "invalidTransaction"; - jvResult[jss::error_exception] = e.what(); + std::shared_ptr stpTrans; + try + { + stpTrans = std::make_shared(std::move(parsed.object.value())); + } + catch (std::exception& e) + { + jvResult[jss::error] = "invalidTransaction"; + jvResult[jss::error_exception] = e.what(); - return jvResult; - } + return jvResult; } std::string reason; auto tpTrans = std::make_shared(stpTrans, reason, context.app); + // Actually run the transaction through the transaction processor try { - // we check before adding to the batch ApplyFlags flags = tapDRY_RUN; // Process the transaction From 26f3bb66aae359e27053cbd0a368a1db639b684b Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 10 Jul 2024 17:59:39 -0400 Subject: [PATCH 11/42] cleanup --- src/xrpld/rpc/handlers/Simulate.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index 08aaadcba0f..dcb1075003a 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -82,7 +82,7 @@ autofillTx(Json::Value& tx_json, RPC::JsonContext& context) if (!hasTicketSeq && !sle) { JLOG(context.app.journal("Simulate").debug()) - << "simulate: Failed to find source account " + << "Failed to find source account " << "in current ledger: " << toBase58(srcAddressID); return rpcError(rpcSRC_ACT_NOT_FOUND); From 3b4b35467ce9fb4cb52815508df5cdfe97bd20ec Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 11 Jul 2024 14:18:07 -0400 Subject: [PATCH 12/42] simplify transaction engine exceptions --- src/xrpld/app/tx/detail/Transactor.cpp | 37 +++++++++++++++----------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index 455d9987967..ec3f4a7d2f2 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -100,22 +100,19 @@ preflight1(PreflightContext const& ctx) } // No point in going any further if the transaction fee is malformed. - if (!(ctx.flags & tapDRY_RUN)) + auto const fee = ctx.tx.getFieldAmount(sfFee); + if (!fee.native() || fee.negative() || !isLegalAmount(fee.xrp())) { - auto const fee = ctx.tx.getFieldAmount(sfFee); - if (!fee.native() || fee.negative() || !isLegalAmount(fee.xrp())) - { - JLOG(ctx.j.debug()) << "preflight1: invalid fee"; - return temBAD_FEE; - } + JLOG(ctx.j.debug()) << "preflight1: invalid fee"; + return temBAD_FEE; + } - auto const spk = ctx.tx.getSigningPubKey(); + auto const spk = ctx.tx.getSigningPubKey(); - if (!spk.empty() && !publicKeyType(makeSlice(spk))) - { - JLOG(ctx.j.debug()) << "preflight1: invalid signing key"; - return temBAD_SIGNATURE; - } + if (!spk.empty() && !publicKeyType(makeSlice(spk))) + { + JLOG(ctx.j.debug()) << "preflight1: invalid signing key"; + return temBAD_SIGNATURE; } // An AccountTxnID field constrains transaction ordering more than the @@ -136,7 +133,13 @@ NotTEC preflight2(PreflightContext const& ctx) { if (ctx.flags & tapDRY_RUN) - return tesSUCCESS; + { + if (ctx.tx.getSigningPubKey().empty() && ctx.tx.getSignature().empty()) + return tesSUCCESS; + // NOTE: This code should never be hit because it's checked in the + // `simulate` RPC + return temINVALID; + } auto const sigValid = checkValidity( ctx.app.getHashRouter(), ctx.tx, ctx.rules, ctx.app.config()); if (sigValid.first == Validity::SigBad) @@ -487,8 +490,10 @@ Transactor::checkSign(PreclaimContext const& ctx) { if (ctx.flags & tapDRY_RUN) { - // TODO: ensure there is no signature included - return tesSUCCESS; + if (ctx.tx.getSigningPubKey().empty() && ctx.tx.getSignature().empty()) + return tesSUCCESS; + // Already checked in preflight2, should never be hit + return temINVALID; } // If the pk is empty, then we must be multi-signing. if (ctx.tx.getSigningPubKey().empty()) From 5c383a913570200e3af7326027e429abd7f7fc57 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 11 Jul 2024 14:45:20 -0400 Subject: [PATCH 13/42] fix rebase issues --- src/test/rpc/Simulate_test.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp index 722b50f7c40..b484a8e4609 100644 --- a/src/test/rpc/Simulate_test.cpp +++ b/src/test/rpc/Simulate_test.cpp @@ -17,15 +17,15 @@ */ //============================================================================== -#include -#include -#include -#include -#include -#include #include #include #include +#include +#include +#include +#include +#include +#include #include #include From b6bf4c50140b42af7b393864ec61322283bce019 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 11 Jul 2024 15:15:58 -0400 Subject: [PATCH 14/42] fix clang-format --- src/xrpld/app/misc/TxQ.h | 2 +- src/xrpld/app/tx/apply.h | 2 +- src/xrpld/rpc/detail/TransactionSign.h | 2 +- src/xrpld/rpc/handlers/Simulate.cpp | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/xrpld/app/misc/TxQ.h b/src/xrpld/app/misc/TxQ.h index e7c2983238f..d139af70d92 100644 --- a/src/xrpld/app/misc/TxQ.h +++ b/src/xrpld/app/misc/TxQ.h @@ -21,13 +21,13 @@ #define RIPPLE_TXQ_H_INCLUDED #include +#include #include #include #include #include #include #include -#include #include #include #include diff --git a/src/xrpld/app/tx/apply.h b/src/xrpld/app/tx/apply.h index b00d47dec26..c16397d8996 100644 --- a/src/xrpld/app/tx/apply.h +++ b/src/xrpld/app/tx/apply.h @@ -20,12 +20,12 @@ #ifndef RIPPLE_TX_APPLY_H_INCLUDED #define RIPPLE_TX_APPLY_H_INCLUDED +#include #include #include #include #include #include -#include #include #include diff --git a/src/xrpld/rpc/detail/TransactionSign.h b/src/xrpld/rpc/detail/TransactionSign.h index 59c200e58d4..43077264d74 100644 --- a/src/xrpld/rpc/detail/TransactionSign.h +++ b/src/xrpld/rpc/detail/TransactionSign.h @@ -20,10 +20,10 @@ #ifndef RIPPLE_RPC_TRANSACTIONSIGN_H_INCLUDED #define RIPPLE_RPC_TRANSACTIONSIGN_H_INCLUDED +#include #include #include #include -#include #include namespace ripple { diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index dcb1075003a..6f63a854d9a 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -22,14 +22,14 @@ #include #include #include -#include -#include -#include #include #include #include #include +#include +#include #include +#include namespace ripple { From d70393506ec97934dd84bbbb74e4294596c411cb Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Mon, 15 Jul 2024 20:00:39 -0400 Subject: [PATCH 15/42] add ledger_index --- src/xrpld/rpc/handlers/Simulate.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index 6f63a854d9a..86cde1916c8 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -188,6 +188,7 @@ doSimulate(RPC::JsonContext& context) context.app, view, tpTrans->getSTransaction(), flags, context.j); jvResult[jss::applied] = result.second; + jvResult[jss::ledger_index] = view.seq(); const bool isBinaryOutput = context.params.get(jss::binary, false).asBool(); From 4489fae1b94f33d2e2ac6ee8a6691ee115ecf2bb Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Mon, 15 Jul 2024 21:20:37 -0700 Subject: [PATCH 16/42] update API changelog --- API-CHANGELOG.md | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md index 58ceed0cde1..266227497b0 100644 --- a/API-CHANGELOG.md +++ b/API-CHANGELOG.md @@ -26,15 +26,25 @@ In `api_version: 2`, the `signer_lists` field [will be moved](#modifications-to- The `network_id` field was added in the `server_info` response in version 1.5.0 (2019), but it is not returned in [reporting mode](https://xrpl.org/rippled-server-modes.html#reporting-mode). -## XRP Ledger server version 2.0.0 +## XRP Ledger server version 2.3.0 + +### Additions + +Additions are intended to be non-breaking (because they are purely additive). -### Additions in 2.2 +- `simulate`: A new RPC that executes a dry run of a transaction submission without actually submitting it to the network for inclusion. + +## XRP Ledger server version 2.2.0 + +### Additions Additions are intended to be non-breaking (because they are purely additive). - `feature`: A non-admin mode that uses the same formatting as admin RPC, but hides potentially-sensitive data. -### Additions in 2.0 +## XRP Ledger server version 2.0.0 + +### Additions Additions are intended to be non-breaking (because they are purely additive). @@ -46,7 +56,7 @@ Additions are intended to be non-breaking (because they are purely additive). [Version 1.12.0](https://github.com/XRPLF/rippled/releases/tag/1.12.0) was released on Sep 6, 2023. -### Additions in 1.12 +### Additions Additions are intended to be non-breaking (because they are purely additive). @@ -88,7 +98,7 @@ Additions are intended to be non-breaking (because they are purely additive). [Version 1.11.0](https://github.com/XRPLF/rippled/releases/tag/1.11.0) was released on Jun 20, 2023. -### Breaking changes in 1.11 +### Breaking changes - Added the ability to mark amendments as obsolete. For the `feature` admin API, there is a new possible value for the `vetoed` field. (https://github.com/XRPLF/rippled/pull/4291) - The value of `vetoed` can now be `true`, `false`, or `"Obsolete"`. @@ -107,7 +117,7 @@ Additions are intended to be non-breaking (because they are purely additive). - `telREQUIRES_NETWORK_ID`: a `NetworkID` is required, but is not present. Add the field to the transaction, and try again. - `telWRONG_NETWORK`: a `NetworkID` is specified, but it is for a different network. Submit the transaction to a different server which is connected to the correct network. -### Additions and bug fixes in 1.11 +### Additions and bug fixes - Added `nftoken_id`, `nftoken_ids` and `offer_id` meta fields into NFT `tx` and `account_tx` responses. (https://github.com/XRPLF/rippled/pull/4447) - Added an `account_flags` object to the `account_info` method response. (https://github.com/XRPLF/rippled/pull/4459) @@ -119,7 +129,7 @@ Additions are intended to be non-breaking (because they are purely additive). [Version 1.10.0](https://github.com/XRPLF/rippled/releases/tag/1.10.0) was released on Mar 14, 2023. -### Breaking changes in 1.10 +### Breaking changes - If the `XRPFees` feature is enabled, the `fee_ref` field will be removed from the [ledger subscription stream](https://xrpl.org/subscribe.html#ledger-stream), because it will no longer From 449b2c67f93af7011d51216741a891f1d49d4c24 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Mon, 15 Jul 2024 22:24:41 -0700 Subject: [PATCH 17/42] better param validation --- include/xrpl/protocol/ErrorCodes.h | 6 ++- src/libxrpl/protocol/ErrorCodes.cpp | 3 +- src/test/rpc/Simulate_test.cpp | 28 ++++++++++++-- src/xrpld/rpc/handlers/Simulate.cpp | 58 ++++++++++++++++++++--------- 4 files changed, 71 insertions(+), 24 deletions(-) diff --git a/include/xrpl/protocol/ErrorCodes.h b/include/xrpl/protocol/ErrorCodes.h index 6d5590ec605..a8aa36f55a6 100644 --- a/include/xrpl/protocol/ErrorCodes.h +++ b/include/xrpl/protocol/ErrorCodes.h @@ -148,8 +148,10 @@ enum error_code_i { // Oracle rpcORACLE_MALFORMED = 94, - rpcLAST = - rpcORACLE_MALFORMED // rpcLAST should always equal the last code.= + // Simulate + rpcTX_SIGNED = 95, + + rpcLAST = rpcTX_SIGNED // rpcLAST should always equal the last code.= }; /** Codes returned in the `warnings` array of certain RPC commands. diff --git a/src/libxrpl/protocol/ErrorCodes.cpp b/src/libxrpl/protocol/ErrorCodes.cpp index 28024fab093..5d78c8628b1 100644 --- a/src/libxrpl/protocol/ErrorCodes.cpp +++ b/src/libxrpl/protocol/ErrorCodes.cpp @@ -110,7 +110,8 @@ constexpr static ErrorInfo unorderedErrorInfos[]{ {rpcTOO_BUSY, "tooBusy", "The server is too busy to help you now.", 503}, {rpcTXN_NOT_FOUND, "txnNotFound", "Transaction not found.", 404}, {rpcUNKNOWN_COMMAND, "unknownCmd", "Unknown method.", 405}, - {rpcORACLE_MALFORMED, "oracleMalformed", "Oracle request is malformed.", 400}}; + {rpcORACLE_MALFORMED, "oracleMalformed", "Oracle request is malformed.", 400}, + {rpcTX_SIGNED, "transactionSigned", "Transaction should not be signed.", 400}}; // clang-format on // Sort and validate unorderedErrorInfos at compile time. Should be diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp index b484a8e4609..cb54af2449f 100644 --- a/src/test/rpc/Simulate_test.cpp +++ b/src/test/rpc/Simulate_test.cpp @@ -54,11 +54,12 @@ class Simulate_test : public beast::unit_test::suite Env env(*this); { + // No params auto resp = env.rpc("json", "simulate"); BEAST_EXPECT(resp[jss::error_message] == "Syntax error."); } - { + // Providing both `tx_json` and `tx_blob` Json::Value params; params[jss::tx_json] = Json::objectValue; params[jss::tx_blob] = "1200"; @@ -67,14 +68,35 @@ class Simulate_test : public beast::unit_test::suite BEAST_EXPECT( resp[jss::result][jss::error_message] == "Invalid parameters."); } - { + // `binary` isn't a boolean Json::Value params; params[jss::tx_blob] = "1200"; params[jss::binary] = "100"; auto resp = env.rpc("json", "simulate", to_string(params)); BEAST_EXPECT( - resp[jss::result][jss::error_message] == "Invalid parameters."); + resp[jss::result][jss::error_message] == + "Invalid field 'binary'."); + } + { + // Empty `tx_json` + Json::Value params; + params[jss::tx_json] = Json::objectValue; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Missing field 'tx.TransactionType'."); + } + { + // Empty `tx_blob` + Json::Value params; + params[jss::tx_blob] = ""; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Invalid field 'tx_blob'."); } } void diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index 86cde1916c8..3f0f4adf1b9 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -38,6 +38,7 @@ autofillTx(Json::Value& tx_json, RPC::JsonContext& context) { if (!tx_json.isMember(jss::Fee)) { + // autofill Fee tx_json[jss::Fee] = RPC::getCurrentFee( context.role, context.app.config(), @@ -47,34 +48,39 @@ autofillTx(Json::Value& tx_json, RPC::JsonContext& context) } if (!tx_json.isMember(sfSigningPubKey.jsonName)) { + // autofill SigningPubKey tx_json[sfSigningPubKey.jsonName] = ""; } else if (tx_json[sfSigningPubKey.jsonName] != "") { - return RPC::make_error( - rpcINVALID_PARAMS, - RPC::invalid_field_message("tx_json.SigningPubKey")); + // Transaction must not be signed + return rpcError(rpcTX_SIGNED); } if (!tx_json.isMember(sfTxnSignature.jsonName)) { + // autofill TxnSignature tx_json[sfTxnSignature.jsonName] = ""; } else if (tx_json[sfTxnSignature.jsonName] != "") { - return RPC::make_error( - rpcINVALID_PARAMS, - RPC::invalid_field_message("tx_json.TxnSignature")); + // Transaction must not be signed + return rpcError(rpcTX_SIGNED); } if (!tx_json.isMember(jss::Sequence)) { + // autofill Sequence bool const hasTicketSeq = tx_json.isMember(sfTicketSequence.jsonName); + auto const accountStr = tx_json[jss::Account]; + if (!accountStr.isString()) + { + return RPC::invalid_field_error("tx.Account"); + } auto const srcAddressID = *(parseBase58(tx_json[jss::Account].asString())); if (!srcAddressID) { return RPC::make_error( - rpcSRC_ACT_MALFORMED, - RPC::invalid_field_message("tx_json.Account")); + rpcSRC_ACT_MALFORMED, RPC::invalid_field_message("tx.Account")); } std::shared_ptr sle = context.app.openLedger().current()->read( @@ -115,7 +121,7 @@ doSimulate(RPC::JsonContext& context) auto const binary = context.params[jss::binary]; if (!binary.isBool()) { - return rpcError(rpcINVALID_PARAMS); + return RPC::invalid_field_error(jss::binary); } } @@ -126,26 +132,42 @@ doSimulate(RPC::JsonContext& context) // both `tx_blob` and `tx_json` included return rpcError(rpcINVALID_PARAMS); } - - auto unHexed = strUnHex(context.params[jss::tx_blob].asString()); + auto const blob = context.params[jss::tx_blob]; + if (!blob.isString()) + { + return RPC::invalid_field_message(jss::tx_blob); + } + auto unHexed = strUnHex(blob.asString()); if (!unHexed || !unHexed->size()) - return RPC::make_error( - rpcINVALID_PARAMS, RPC::invalid_field_message("tx_blob")); + return RPC::invalid_field_error(jss::tx_blob); SerialIter sitTrans(makeSlice(*unHexed)); tx_json = STObject(std::ref(sitTrans), sfGeneric).getJson(JsonOptions::none); } - else + else if (context.params.isMember(jss::tx_json)) { - if (!context.params.isMember(jss::tx_json)) + tx_json = context.params[jss::tx_json]; + if (!tx_json.isObject()) { - // neither `tx_blob` nor `tx_json` included` - return rpcError(rpcINVALID_PARAMS); + return RPC::object_field_error(jss::tx_json); } + } + else + { + // neither `tx_blob` nor `tx_json` included` + return rpcError(rpcINVALID_PARAMS); + } - tx_json = context.params[jss::tx_json]; + // basic sanity checks for transaction shape + if (!tx_json.isMember(jss::TransactionType)) + { + return RPC::missing_field_error("tx.TransactionType"); + } + if (!tx_json.isMember(jss::Account)) + { + return RPC::missing_field_error("tx.Account"); } // autofill fields if they're not included (e.g. `Fee`, `Sequence`) From fa246bb3f0f58cc63990d26b5a170b84aef7620b Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 16 Jul 2024 22:11:09 -0700 Subject: [PATCH 18/42] add tests --- src/test/rpc/Simulate_test.cpp | 317 ++++++++++++++++++++++++---- src/xrpld/rpc/handlers/Simulate.cpp | 13 +- 2 files changed, 287 insertions(+), 43 deletions(-) diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp index cb54af2449f..bf6101e8983 100644 --- a/src/test/rpc/Simulate_test.cpp +++ b/src/test/rpc/Simulate_test.cpp @@ -37,21 +37,36 @@ namespace test { class Simulate_test : public beast::unit_test::suite { void - checkBasicReturnValidity(Json::Value result) + checkBasicReturnValidity(Json::Value result, Json::Value tx) { BEAST_EXPECT(result[jss::applied] == false); BEAST_EXPECT(result.isMember(jss::engine_result)); BEAST_EXPECT(result.isMember(jss::engine_result_code)); BEAST_EXPECT(result.isMember(jss::engine_result_message)); + + if (BEAST_EXPECT(result.isMember(jss::tx_json))) + { + auto const tx_json = result[jss::tx_json]; + BEAST_EXPECT( + tx_json[jss::TransactionType] == tx[jss::TransactionType]); + BEAST_EXPECT(tx_json[jss::Account] == tx[jss::Account]); + BEAST_EXPECT( + tx_json[jss::SigningPubKey] == tx.get(jss::SigningPubKey, "")); + BEAST_EXPECT( + tx_json[jss::TxnSignature] == tx.get(jss::TxnSignature, "")); + BEAST_EXPECT(tx_json[jss::Fee] == tx.get(jss::Fee, "10")); + BEAST_EXPECT(tx_json[jss::Sequence] == tx.get(jss::Sequence, 1)); + } } void testParamErrors() { - testcase("Test errors in the parameters"); + testcase("Test parameter errors"); using namespace jtx; Env env(*this); + Account const alice("alice"); { // No params @@ -60,7 +75,7 @@ class Simulate_test : public beast::unit_test::suite } { // Providing both `tx_json` and `tx_blob` - Json::Value params; + Json::Value params = Json::objectValue; params[jss::tx_json] = Json::objectValue; params[jss::tx_blob] = "1200"; @@ -70,7 +85,7 @@ class Simulate_test : public beast::unit_test::suite } { // `binary` isn't a boolean - Json::Value params; + Json::Value params = Json::objectValue; params[jss::tx_blob] = "1200"; params[jss::binary] = "100"; auto resp = env.rpc("json", "simulate", to_string(params)); @@ -80,7 +95,7 @@ class Simulate_test : public beast::unit_test::suite } { // Empty `tx_json` - Json::Value params; + Json::Value params = Json::objectValue; params[jss::tx_json] = Json::objectValue; auto resp = env.rpc("json", "simulate", to_string(params)); @@ -88,9 +103,21 @@ class Simulate_test : public beast::unit_test::suite resp[jss::result][jss::error_message] == "Missing field 'tx.TransactionType'."); } + { + // No tx.account + Json::Value params = Json::objectValue; + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::Payment; + params[jss::tx_json] = tx_json; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Missing field 'tx.Account'."); + } { // Empty `tx_blob` - Json::Value params; + Json::Value params = Json::objectValue; params[jss::tx_blob] = ""; auto resp = env.rpc("json", "simulate", to_string(params)); @@ -98,62 +125,276 @@ class Simulate_test : public beast::unit_test::suite resp[jss::result][jss::error_message] == "Invalid field 'tx_blob'."); } + { + // Non-string `tx_blob` + Json::Value params; + params[jss::tx_blob] = 1.1; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Invalid field 'tx_blob'."); + } + { + // Non-object `tx_json` + Json::Value params = Json::objectValue; + params[jss::tx_json] = ""; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Invalid field 'tx_json', not object."); + } + { + // Invalid transaction + Json::Value params = Json::objectValue; + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::Payment; + tx_json[jss::Account] = env.master.human(); + params[jss::tx_json] = tx_json; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_exception] == + "Field 'Destination' is required but missing."); + } + { + // Bad account + Json::Value params; + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::AccountSet; + tx_json[jss::Account] = "badAccount"; + params[jss::tx_json] = tx_json; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT(resp[jss::result][jss::error] == "srcActMalformed"); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Invalid field 'tx.Account'."); + } + { + // Account doesn't exist for Sequence autofill + Json::Value params; + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::AccountSet; + tx_json[jss::Account] = alice.human(); + params[jss::tx_json] = tx_json; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Source account not found."); + } + { + // Invalid transaction + Json::Value params; + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::AccountSet; + tx_json[jss::Account] = alice.human(); + tx_json[jss::tx_json] = jss::AccountSet; + params[jss::tx_json] = tx_json; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Source account not found."); + } } void - testBasic() + testSuccessfulTransaction() { - testcase("Basic test"); + testcase("Successful transaction"); using namespace jtx; Env env(*this); + static auto const newDomain = "123ABC"; { - Json::Value tx; + auto testTx = [&](Json::Value tx) { + Json::Value params; + params[jss::tx_json] = tx; + + auto resp = env.rpc("json", "simulate", to_string(params)); + auto result = resp[jss::result]; + checkBasicReturnValidity(result, tx); - auto newDomain = "123ABC"; + BEAST_EXPECT(result[jss::engine_result] == "tesSUCCESS"); + BEAST_EXPECT(result[jss::engine_result_code] == 0); + BEAST_EXPECT( + result[jss::engine_result_message] == + "The simulated transaction would have been applied."); + + if (BEAST_EXPECT(result.isMember(jss::metadata))) + { + auto metadata = result[jss::metadata]; + if (BEAST_EXPECT( + metadata.isMember(sfAffectedNodes.jsonName))) + { + BEAST_EXPECT( + metadata[sfAffectedNodes.jsonName].size() == 1); + auto node = metadata[sfAffectedNodes.jsonName][0u]; + if (BEAST_EXPECT( + node.isMember(sfModifiedNode.jsonName))) + { + auto modifiedNode = node[sfModifiedNode.jsonName]; + BEAST_EXPECT( + modifiedNode[sfLedgerEntryType.jsonName] == + "AccountRoot"); + auto finalFields = + modifiedNode[sfFinalFields.jsonName]; + BEAST_EXPECT( + finalFields[sfDomain.jsonName] == newDomain); + } + } + BEAST_EXPECT(metadata[sfTransactionIndex.jsonName] == 0); + BEAST_EXPECT( + metadata[sfTransactionResult.jsonName] == "tesSUCCESS"); + } + }; + + Json::Value tx; tx[jss::Account] = Account::master.human(); tx[jss::TransactionType] = jss::AccountSet; tx[sfDomain.jsonName] = newDomain; + + // test with autofill + testTx(tx); + tx[sfSigningPubKey.jsonName] = ""; tx[sfTxnSignature.jsonName] = ""; tx[sfSequence.jsonName] = 1; tx[sfFee.jsonName] = "12"; - Json::Value params; - params[jss::tx_json] = tx; + // test without autofill + testTx(tx); - auto resp = env.rpc("json", "simulate", to_string(params)); - auto result = resp[jss::result]; - checkBasicReturnValidity(result); + // TODO: check that the ledger wasn't affected + } + } - BEAST_EXPECT(result[jss::engine_result] == "tesSUCCESS"); - BEAST_EXPECT(result[jss::engine_result_code] == 0); - BEAST_EXPECT( - result[jss::engine_result_message] == - "The simulated transaction would have been applied."); + void + testTransactionNonTecFailure() + { + testcase("Transaction non-tec failure"); + + using namespace jtx; + Env env(*this); + Account const alice("alice"); + + { + auto testTx = [&](Json::Value tx) { + Json::Value params; + params[jss::tx_json] = tx; + + auto resp = env.rpc("json", "simulate", to_string(params)); + auto result = resp[jss::result]; + checkBasicReturnValidity(result, tx); + + BEAST_EXPECT(result[jss::engine_result] == "temBAD_AMOUNT"); + BEAST_EXPECT(result[jss::engine_result_code] == -298); + BEAST_EXPECT( + result[jss::engine_result_message] == + "Can only send positive amounts."); + + BEAST_EXPECT(!result.isMember(jss::metadata)); + }; + + Json::Value tx; + + tx[jss::Account] = Account::master.human(); + tx[jss::TransactionType] = jss::Payment; + tx[sfDestination.jsonName] = alice.human(); + tx[sfAmount.jsonName] = "0"; // invalid amount + + // test with autofill + testTx(tx); + + tx[sfSigningPubKey.jsonName] = ""; + tx[sfTxnSignature.jsonName] = ""; + tx[sfSequence.jsonName] = 1; + tx[sfFee.jsonName] = "12"; + + // test without autofill + testTx(tx); + + // TODO: check that the ledger wasn't affected + } + } + + void + testTransactionTecFailure() + { + testcase("Transaction tec failure"); + + using namespace jtx; + Env env(*this); + Account const alice("alice"); + + { + auto testTx = [&](Json::Value tx) { + Json::Value params; + params[jss::tx_json] = tx; + + auto resp = env.rpc("json", "simulate", to_string(params)); + auto result = resp[jss::result]; + checkBasicReturnValidity(result, tx); - if (BEAST_EXPECT(result.isMember(jss::metadata))) - { - auto metadata = result[jss::metadata]; - if (BEAST_EXPECT(metadata.isMember(sfAffectedNodes.jsonName))) + BEAST_EXPECT( + result[jss::engine_result] == "tecNO_DST_INSUF_XRP"); + BEAST_EXPECT(result[jss::engine_result_code] == 125); + BEAST_EXPECT( + result[jss::engine_result_message] == + "Destination does not exist. Too little XRP sent to create " + "it."); + + if (BEAST_EXPECT(result.isMember(jss::metadata))) { - auto node = metadata[sfAffectedNodes.jsonName][0u]; - if (BEAST_EXPECT(node.isMember(sfModifiedNode.jsonName))) + auto metadata = result[jss::metadata]; + if (BEAST_EXPECT( + metadata.isMember(sfAffectedNodes.jsonName))) { - auto modifiedNode = node[sfModifiedNode.jsonName]; - BEAST_EXPECT( - modifiedNode[sfLedgerEntryType.jsonName] == - "AccountRoot"); - auto finalFields = modifiedNode[sfFinalFields.jsonName]; BEAST_EXPECT( - finalFields[sfDomain.jsonName] == newDomain); + metadata[sfAffectedNodes.jsonName].size() == 1); + auto node = metadata[sfAffectedNodes.jsonName][0u]; + if (BEAST_EXPECT( + node.isMember(sfModifiedNode.jsonName))) + { + auto modifiedNode = node[sfModifiedNode.jsonName]; + BEAST_EXPECT( + modifiedNode[sfLedgerEntryType.jsonName] == + "AccountRoot"); + auto finalFields = + modifiedNode[sfFinalFields.jsonName]; + BEAST_EXPECT( + finalFields[sfBalance.jsonName] == + "99999999999999990"); + } } + BEAST_EXPECT(metadata[sfTransactionIndex.jsonName] == 0); + BEAST_EXPECT( + metadata[sfTransactionResult.jsonName] == + "tecNO_DST_INSUF_XRP"); } - BEAST_EXPECT(metadata[sfTransactionIndex.jsonName] == 0); - BEAST_EXPECT( - metadata[sfTransactionResult.jsonName] == "tesSUCCESS"); - } + }; + + Json::Value tx; + + tx[jss::Account] = Account::master.human(); + tx[jss::TransactionType] = jss::Payment; + tx[sfDestination.jsonName] = alice.human(); + tx[sfAmount.jsonName] = "1"; // not enough to create an account + + // test with autofill + testTx(tx); + + tx[sfSigningPubKey.jsonName] = ""; + tx[sfTxnSignature.jsonName] = ""; + tx[sfSequence.jsonName] = 1; + tx[sfFee.jsonName] = "10"; + + // test without autofill + testTx(tx); // TODO: check that the ledger wasn't affected } @@ -164,7 +405,9 @@ class Simulate_test : public beast::unit_test::suite run() override { testParamErrors(); - testBasic(); + testSuccessfulTransaction(); + testTransactionNonTecFailure(); + testTransactionTecFailure(); } }; diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index 3f0f4adf1b9..e3a95673d69 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -73,23 +73,24 @@ autofillTx(Json::Value& tx_json, RPC::JsonContext& context) auto const accountStr = tx_json[jss::Account]; if (!accountStr.isString()) { + // sanity check, should fail earlier return RPC::invalid_field_error("tx.Account"); } auto const srcAddressID = - *(parseBase58(tx_json[jss::Account].asString())); - if (!srcAddressID) + parseBase58(tx_json[jss::Account].asString()); + if (!srcAddressID.has_value()) { return RPC::make_error( rpcSRC_ACT_MALFORMED, RPC::invalid_field_message("tx.Account")); } std::shared_ptr sle = context.app.openLedger().current()->read( - keylet::account(srcAddressID)); + keylet::account(*srcAddressID)); if (!hasTicketSeq && !sle) { JLOG(context.app.journal("Simulate").debug()) << "Failed to find source account " - << "in current ledger: " << toBase58(srcAddressID); + << "in current ledger: " << toBase58(*srcAddressID); return rpcError(rpcSRC_ACT_NOT_FOUND); } @@ -135,7 +136,7 @@ doSimulate(RPC::JsonContext& context) auto const blob = context.params[jss::tx_blob]; if (!blob.isString()) { - return RPC::invalid_field_message(jss::tx_blob); + return RPC::invalid_field_error(jss::tx_blob); } auto unHexed = strUnHex(blob.asString()); @@ -156,7 +157,7 @@ doSimulate(RPC::JsonContext& context) } else { - // neither `tx_blob` nor `tx_json` included` + // neither `tx_blob` nor `tx_json` included return rpcError(rpcINVALID_PARAMS); } From 3fadb8d197acea12ba6c9ddf05438e493b6547ec Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 16 Jul 2024 22:36:22 -0700 Subject: [PATCH 19/42] add command line version --- src/xrpld/app/main/Main.cpp | 1 + src/xrpld/net/detail/RPCCall.cpp | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/xrpld/app/main/Main.cpp b/src/xrpld/app/main/Main.cpp index 059d9758d39..0ee0efa9f6d 100644 --- a/src/xrpld/app/main/Main.cpp +++ b/src/xrpld/app/main/Main.cpp @@ -176,6 +176,7 @@ printHelp(const po::options_description& desc) " sign_for " "[offline]\n" " stop\n" + " simulate [|] []\n" " submit |[ ]\n" " submit_multisigned \n" " tx \n" diff --git a/src/xrpld/net/detail/RPCCall.cpp b/src/xrpld/net/detail/RPCCall.cpp index 533878ab1b0..15fa38be0c9 100644 --- a/src/xrpld/net/detail/RPCCall.cpp +++ b/src/xrpld/net/detail/RPCCall.cpp @@ -924,6 +924,37 @@ class RPCParser return rpcError(rpcINVALID_PARAMS); } + // simulate any transaction on the network + // + // simulate [binary] + // simulate [binary] + Json::Value + parseSimulate(Json::Value const& jvParams) + { + Json::Value txJSON; + Json::Reader reader; + Json::Value jvRequest{Json::objectValue}; + + if (!jvParams[0u].isString()) + return rpcError(rpcINVALID_PARAMS); + if (reader.parse(jvParams[0u].asString(), txJSON)) + { + jvRequest[jss::tx_json] = txJSON; + } + else + { + jvRequest[jss::tx_blob] = jvParams[0u].asString(); + } + if (jvParams.size() == 2) + { + if (!jvParams[1u].isString() || jvParams[1u].asString() != "binary") + return rpcError(rpcINVALID_PARAMS); + jvRequest[jss::binary] = true; + } + + return jvRequest; + } + // sign/submit any transaction to the network // // sign offline @@ -1244,6 +1275,7 @@ class RPCParser {"sign", &RPCParser::parseSignSubmit, 2, 3}, {"sign_for", &RPCParser::parseSignFor, 3, 4}, {"stop", &RPCParser::parseAsIs, 0, 0}, + {"simulate", &RPCParser::parseSimulate, 1, 2}, {"submit", &RPCParser::parseSignSubmit, 1, 3}, {"submit_multisigned", &RPCParser::parseSubmitMultiSigned, 1, 1}, {"transaction_entry", &RPCParser::parseTransactionEntry, 2, 2}, From 08b65d0bc4f535d66a045644824d1f772e1ad2e5 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 18 Jul 2024 10:36:04 -0700 Subject: [PATCH 20/42] rename params --- src/test/app/PseudoTx_test.cpp | 4 ++-- src/test/app/Regression_test.cpp | 8 ++++---- src/test/app/TxQ_test.cpp | 12 ++++++------ src/test/consensus/NegativeUNL_test.cpp | 4 ++-- src/xrpld/app/ledger/detail/LedgerMaster.cpp | 2 +- src/xrpld/app/ledger/detail/OpenLedger.cpp | 6 +++--- src/xrpld/app/misc/NetworkOPs.cpp | 6 +++--- src/xrpld/app/misc/detail/TxQ.cpp | 12 ++++++------ src/xrpld/app/tx/applySteps.h | 4 ++-- src/xrpld/app/tx/detail/apply.cpp | 12 ++++++------ src/xrpld/rpc/handlers/Simulate.cpp | 6 +++--- 11 files changed, 38 insertions(+), 38 deletions(-) diff --git a/src/test/app/PseudoTx_test.cpp b/src/test/app/PseudoTx_test.cpp index 238041a17bc..a04316cbd7b 100644 --- a/src/test/app/PseudoTx_test.cpp +++ b/src/test/app/PseudoTx_test.cpp @@ -92,8 +92,8 @@ struct PseudoTx_test : public beast::unit_test::suite [&](OpenView& view, beast::Journal j) { auto const result = ripple::apply(env.app(), view, stx, tapNONE, j); - BEAST_EXPECT(!result.second && result.first == temINVALID); - return result.second; + BEAST_EXPECT(!result.applied && result.ter == temINVALID); + return result.applied; }); } } diff --git a/src/test/app/Regression_test.cpp b/src/test/app/Regression_test.cpp index f54a88ace00..baad43de401 100644 --- a/src/test/app/Regression_test.cpp +++ b/src/test/app/Regression_test.cpp @@ -76,8 +76,8 @@ struct Regression_test : public beast::unit_test::suite auto const result = ripple::apply(env.app(), accum, *jt.stx, tapNONE, env.journal); - BEAST_EXPECT(result.first == tesSUCCESS); - BEAST_EXPECT(result.second); + BEAST_EXPECT(result.ter == tesSUCCESS); + BEAST_EXPECT(result.applied); accum.apply(*next); } @@ -100,8 +100,8 @@ struct Regression_test : public beast::unit_test::suite auto const result = ripple::apply(env.app(), accum, *jt.stx, tapNONE, env.journal); - BEAST_EXPECT(result.first == tecINSUFF_FEE); - BEAST_EXPECT(result.second); + BEAST_EXPECT(result.ter == tecINSUFF_FEE); + BEAST_EXPECT(result.applied); accum.apply(*next); } diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index 45926a37606..07670cd8d1e 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -1064,8 +1064,8 @@ class TxQPosNegFlows_test : public beast::unit_test::suite [&](OpenView& view, beast::Journal j) { auto result = ripple::apply( env.app(), view, *jt.stx, tapNONE, env.journal); - parsed.ter = result.first; - return result.second; + parsed.ter = result.ter; + return result.applied; }); env.postconditions(jt, parsed); } @@ -4173,8 +4173,8 @@ class TxQPosNegFlows_test : public beast::unit_test::suite env.jt(noop(alice), seq(aliceSeq), openLedgerFee(env)); auto const result = ripple::apply(env.app(), view, *tx.stx, tapUNLIMITED, j); - BEAST_EXPECT(result.first == tesSUCCESS && result.second); - return result.second; + BEAST_EXPECT(result.ter == tesSUCCESS && result.applied); + return result.applied; }); // the queued transaction is still there checkMetrics(__LINE__, env, 1, std::nullopt, 5, 3, 256); @@ -4245,8 +4245,8 @@ class TxQPosNegFlows_test : public beast::unit_test::suite noop(alice), ticket::use(tktSeq0 + 1), openLedgerFee(env)); auto const result = ripple::apply(env.app(), view, *tx.stx, tapUNLIMITED, j); - BEAST_EXPECT(result.first == tesSUCCESS && result.second); - return result.second; + BEAST_EXPECT(result.ter == tesSUCCESS && result.applied); + return result.applied; }); // the queued transaction is still there checkMetrics(__LINE__, env, 1, std::nullopt, 5, 3, 256); diff --git a/src/test/consensus/NegativeUNL_test.cpp b/src/test/consensus/NegativeUNL_test.cpp index 200cd166031..df63a62c299 100644 --- a/src/test/consensus/NegativeUNL_test.cpp +++ b/src/test/consensus/NegativeUNL_test.cpp @@ -1924,9 +1924,9 @@ applyAndTestResult(jtx::Env& env, OpenView& view, STTx const& tx, bool pass) { auto res = apply(env.app(), view, tx, ApplyFlags::tapNONE, env.journal); if (pass) - return res.first == tesSUCCESS; + return res.ter == tesSUCCESS; else - return res.first == tefFAILURE || res.first == temDISABLED; + return res.ter == tefFAILURE || res.ter == temDISABLED; } bool diff --git a/src/xrpld/app/ledger/detail/LedgerMaster.cpp b/src/xrpld/app/ledger/detail/LedgerMaster.cpp index f03004fd14c..ece66f87125 100644 --- a/src/xrpld/app/ledger/detail/LedgerMaster.cpp +++ b/src/xrpld/app/ledger/detail/LedgerMaster.cpp @@ -557,7 +557,7 @@ LedgerMaster::applyHeldTransactions() ApplyFlags flags = tapNONE; auto const result = app_.getTxQ().apply(app_, view, it.second, flags, j); - if (result.second) + if (result.applied) any = true; } return any; diff --git a/src/xrpld/app/ledger/detail/OpenLedger.cpp b/src/xrpld/app/ledger/detail/OpenLedger.cpp index 461d98ae4ac..a6b93b46949 100644 --- a/src/xrpld/app/ledger/detail/OpenLedger.cpp +++ b/src/xrpld/app/ledger/detail/OpenLedger.cpp @@ -167,10 +167,10 @@ OpenLedger::apply_one( flags = flags | tapRETRY; // If it's in anybody's proposed set, try to keep it in the ledger auto const result = ripple::apply(app, view, *tx, flags, j); - if (result.second || result.first == terQUEUED) + if (result.applied || result.ter == terQUEUED) return Result::success; - if (isTefFailure(result.first) || isTemMalformed(result.first) || - isTelLocal(result.first)) + if (isTefFailure(result.ter) || isTemMalformed(result.ter) || + isTelLocal(result.ter)) return Result::failure; return Result::retry; } diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index 9cf5d097099..028b00cb639 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -1349,9 +1349,9 @@ NetworkOPsImp::apply(std::unique_lock& batchLock) auto const result = app_.getTxQ().apply( app_, view, e.transaction->getSTransaction(), flags, j); - e.result = result.first; - e.applied = result.second; - changed = changed || result.second; + e.result = result.ter; + e.applied = result.applied; + changed = changed || result.applied; } return changed; }); diff --git a/src/xrpld/app/misc/detail/TxQ.cpp b/src/xrpld/app/misc/detail/TxQ.cpp index af071b6d2a0..a395850fb6b 100644 --- a/src/xrpld/app/misc/detail/TxQ.cpp +++ b/src/xrpld/app/misc/detail/TxQ.cpp @@ -552,7 +552,7 @@ TxQ::tryClearAccountQueueUpThruTx( // succeeds, the MaybeTx will be destructed, so it'll be // moot. --it->second.retriesRemaining; - it->second.lastResult = txResult.first; + it->second.lastResult = txResult.ter; // In TxQ::apply we note that it's possible for a transaction with // a ticket to both be in the queue and in the ledger. And, while @@ -570,20 +570,20 @@ TxQ::tryClearAccountQueueUpThruTx( // transactions in the account queue. Then, if clearing the account // is successful, we will have removed any ticketed transactions // that can never succeed. - if (txResult.first == tefNO_TICKET) + if (txResult.ter == tefNO_TICKET) continue; - if (!txResult.second) + if (!txResult.applied) { // Transaction failed to apply. Fall back to the normal process. - return {txResult.first, false}; + return {txResult.ter, false}; } } // Apply the current tx. Because the state of the view has been changed // by the queued txs, we also need to preclaim again. auto const txResult = doApply(preclaim(pfresult, app, view), app, view); - if (txResult.second) + if (txResult.applied) { // All of the queued transactions applied, so remove them from the // queue. @@ -1195,7 +1195,7 @@ TxQ::apply( flags, metricsSnapshot, j); - if (result.second) + if (result.applied) { sandbox.apply(view); /* Can't erase (*replacedTxIter) here because success diff --git a/src/xrpld/app/tx/applySteps.h b/src/xrpld/app/tx/applySteps.h index 59337725d1d..2ea26b04a5c 100644 --- a/src/xrpld/app/tx/applySteps.h +++ b/src/xrpld/app/tx/applySteps.h @@ -31,8 +31,8 @@ class TxQ; struct TxApplyResult { - TER first; - bool second; + TER ter; + bool applied; std::optional metadata = std::nullopt; }; diff --git a/src/xrpld/app/tx/detail/apply.cpp b/src/xrpld/app/tx/detail/apply.cpp index c7e10224d85..47085671962 100644 --- a/src/xrpld/app/tx/detail/apply.cpp +++ b/src/xrpld/app/tx/detail/apply.cpp @@ -140,23 +140,23 @@ applyTransaction( try { auto const result = apply(app, view, txn, flags, j); - if (result.second) + if (result.applied) { JLOG(j.debug()) - << "Transaction applied: " << transHuman(result.first); + << "Transaction applied: " << transHuman(result.ter); return ApplyResult::Success; } - if (isTefFailure(result.first) || isTemMalformed(result.first) || - isTelLocal(result.first)) + if (isTefFailure(result.ter) || isTemMalformed(result.ter) || + isTelLocal(result.ter)) { // failure JLOG(j.debug()) - << "Transaction failure: " << transHuman(result.first); + << "Transaction failure: " << transHuman(result.ter); return ApplyResult::Fail; } - JLOG(j.debug()) << "Transaction retry: " << transHuman(result.first); + JLOG(j.debug()) << "Transaction retry: " << transHuman(result.ter); return ApplyResult::Retry; } catch (std::exception const& ex) diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index e3a95673d69..80dcf625189 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -210,7 +210,7 @@ doSimulate(RPC::JsonContext& context) auto const result = context.app.getTxQ().apply( context.app, view, tpTrans->getSTransaction(), flags, context.j); - jvResult[jss::applied] = result.second; + jvResult[jss::applied] = result.applied; jvResult[jss::ledger_index] = view.seq(); const bool isBinaryOutput = @@ -219,11 +219,11 @@ doSimulate(RPC::JsonContext& context) // Convert the TER to human-readable values std::string sToken; std::string sHuman; - transResultInfo(result.first, sToken, sHuman); + transResultInfo(result.ter, sToken, sHuman); // Engine result jvResult[jss::engine_result] = sToken; - jvResult[jss::engine_result_code] = result.first; + jvResult[jss::engine_result_code] = result.ter; jvResult[jss::engine_result_message] = sHuman; if (sToken == "tesSUCCESS") { From e217dc15e978a708bda115086241cfda3f10f214 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 18 Jul 2024 11:02:42 -0700 Subject: [PATCH 21/42] better binary/tx_blob testing --- src/test/rpc/Simulate_test.cpp | 121 ++++++++++++++++++++++++++------- 1 file changed, 96 insertions(+), 25 deletions(-) diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp index bf6101e8983..70bd4f3fdad 100644 --- a/src/test/rpc/Simulate_test.cpp +++ b/src/test/rpc/Simulate_test.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -37,26 +38,35 @@ namespace test { class Simulate_test : public beast::unit_test::suite { void - checkBasicReturnValidity(Json::Value result, Json::Value tx) + checkBasicReturnValidity(Json::Value& result, Json::Value& tx) { BEAST_EXPECT(result[jss::applied] == false); BEAST_EXPECT(result.isMember(jss::engine_result)); BEAST_EXPECT(result.isMember(jss::engine_result_code)); BEAST_EXPECT(result.isMember(jss::engine_result_message)); + BEAST_EXPECT( + result.isMember(jss::tx_json) || result.isMember(jss::tx_blob)); - if (BEAST_EXPECT(result.isMember(jss::tx_json))) + Json::Value tx_json; + if (result.isMember(jss::tx_json)) { - auto const tx_json = result[jss::tx_json]; - BEAST_EXPECT( - tx_json[jss::TransactionType] == tx[jss::TransactionType]); - BEAST_EXPECT(tx_json[jss::Account] == tx[jss::Account]); - BEAST_EXPECT( - tx_json[jss::SigningPubKey] == tx.get(jss::SigningPubKey, "")); - BEAST_EXPECT( - tx_json[jss::TxnSignature] == tx.get(jss::TxnSignature, "")); - BEAST_EXPECT(tx_json[jss::Fee] == tx.get(jss::Fee, "10")); - BEAST_EXPECT(tx_json[jss::Sequence] == tx.get(jss::Sequence, 1)); + tx_json = result[jss::tx_json]; + } + else + { + auto unHexed = strUnHex(result[jss::tx_blob].asString()); + SerialIter sitTrans(makeSlice(*unHexed)); + tx_json = STObject(std::ref(sitTrans), sfGeneric) + .getJson(JsonOptions::none); } + BEAST_EXPECT(tx_json[jss::TransactionType] == tx[jss::TransactionType]); + BEAST_EXPECT(tx_json[jss::Account] == tx[jss::Account]); + BEAST_EXPECT( + tx_json[jss::SigningPubKey] == tx.get(jss::SigningPubKey, "")); + BEAST_EXPECT( + tx_json[jss::TxnSignature] == tx.get(jss::TxnSignature, "")); + BEAST_EXPECT(tx_json[jss::Fee] == tx.get(jss::Fee, "10")); + BEAST_EXPECT(tx_json[jss::Sequence] == tx.get(jss::Sequence, 1)); } void @@ -210,10 +220,7 @@ class Simulate_test : public beast::unit_test::suite static auto const newDomain = "123ABC"; { - auto testTx = [&](Json::Value tx) { - Json::Value params; - params[jss::tx_json] = tx; - + auto testSimulation = [&](Json::Value& params, Json::Value& tx) { auto resp = env.rpc("json", "simulate", to_string(params)); auto result = resp[jss::result]; checkBasicReturnValidity(result, tx); @@ -252,6 +259,24 @@ class Simulate_test : public beast::unit_test::suite } }; + auto testTx = [&](Json::Value& tx) { + { + Json::Value params; + params[jss::tx_json] = tx; + testSimulation(params, tx); + } + { + STParsedJSONObject parsed(std::string(jss::tx_json), tx); + if (BEAST_EXPECT(parsed.object.has_value())) + { + Json::Value params; + params[jss::tx_blob] = + strHex(parsed.object->getSerializer().peekData()); + testSimulation(params, tx); + } + } + }; + Json::Value tx; tx[jss::Account] = Account::master.human(); @@ -283,10 +308,7 @@ class Simulate_test : public beast::unit_test::suite Account const alice("alice"); { - auto testTx = [&](Json::Value tx) { - Json::Value params; - params[jss::tx_json] = tx; - + auto testSimulation = [&](Json::Value& params, Json::Value& tx) { auto resp = env.rpc("json", "simulate", to_string(params)); auto result = resp[jss::result]; checkBasicReturnValidity(result, tx); @@ -300,6 +322,26 @@ class Simulate_test : public beast::unit_test::suite BEAST_EXPECT(!result.isMember(jss::metadata)); }; + auto testTx = [&](Json::Value& tx) { + { + Json::Value params; + params[jss::tx_json] = tx; + testSimulation(params, tx); + params[jss::binary] = true; + testSimulation(params, tx); + } + { + STParsedJSONObject parsed(std::string(jss::tx_json), tx); + if (BEAST_EXPECT(parsed.object.has_value())) + { + Json::Value params; + params[jss::tx_blob] = + strHex(parsed.object->getSerializer().peekData()); + testSimulation(params, tx); + } + } + }; + Json::Value tx; tx[jss::Account] = Account::master.human(); @@ -332,10 +374,7 @@ class Simulate_test : public beast::unit_test::suite Account const alice("alice"); { - auto testTx = [&](Json::Value tx) { - Json::Value params; - params[jss::tx_json] = tx; - + auto testSimulation = [&](Json::Value& params, Json::Value& tx) { auto resp = env.rpc("json", "simulate", to_string(params)); auto result = resp[jss::result]; checkBasicReturnValidity(result, tx); @@ -350,7 +389,19 @@ class Simulate_test : public beast::unit_test::suite if (BEAST_EXPECT(result.isMember(jss::metadata))) { - auto metadata = result[jss::metadata]; + Json::Value metadata; + if (params.get(jss::binary, false).asBool()) + { + auto unHexed = + strUnHex(result[jss::metadata].asString()); + SerialIter sitTrans(makeSlice(*unHexed)); + metadata = STObject(std::ref(sitTrans), sfGeneric) + .getJson(JsonOptions::none); + } + else + { + metadata = result[jss::metadata]; + } if (BEAST_EXPECT( metadata.isMember(sfAffectedNodes.jsonName))) { @@ -378,6 +429,26 @@ class Simulate_test : public beast::unit_test::suite } }; + auto testTx = [&](Json::Value& tx) { + { + Json::Value params; + params[jss::tx_json] = tx; + testSimulation(params, tx); + params[jss::binary] = true; + testSimulation(params, tx); + } + { + STParsedJSONObject parsed(std::string(jss::tx_json), tx); + if (BEAST_EXPECT(parsed.object.has_value())) + { + Json::Value params; + params[jss::tx_blob] = + strHex(parsed.object->getSerializer().peekData()); + testSimulation(params, tx); + } + } + }; + Json::Value tx; tx[jss::Account] = Account::master.human(); From d824daf89168203fbc426a5b34ebb0bb48c2022c Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 18 Jul 2024 13:30:40 -0700 Subject: [PATCH 22/42] test CLI version --- src/test/rpc/Simulate_test.cpp | 571 ++++++++++++++++----------------- 1 file changed, 274 insertions(+), 297 deletions(-) diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp index 70bd4f3fdad..16a5812b02a 100644 --- a/src/test/rpc/Simulate_test.cpp +++ b/src/test/rpc/Simulate_test.cpp @@ -70,246 +70,263 @@ class Simulate_test : public beast::unit_test::suite } void - testParamErrors() + testTx( + jtx::Env& env, + Json::Value& tx, + std::function validate){ + {Json::Value params; + params[jss::tx_json] = tx; + validate(env.rpc("json", "simulate", to_string(params)), tx); + params[jss::binary] = true; + validate(env.rpc("json", "simulate", to_string(params)), tx); + validate(env.rpc("simulate", to_string(tx)), tx); + validate(env.rpc("simulate", to_string(tx), "binary"), tx); +} +{ + STParsedJSONObject parsed(std::string(jss::tx_json), tx); + auto const tx_blob = strHex(parsed.object->getSerializer().peekData()); + if (BEAST_EXPECT(parsed.object.has_value())) { - testcase("Test parameter errors"); + Json::Value params; + params[jss::tx_blob] = tx_blob; + validate(env.rpc("json", "simulate", to_string(params)), tx); + params[jss::binary] = true; + validate(env.rpc("json", "simulate", to_string(params)), tx); + } + validate(env.rpc("simulate", tx_blob), tx); + validate(env.rpc("simulate", tx_blob, "binary"), tx); +} +}; // namespace test - using namespace jtx; - Env env(*this); - Account const alice("alice"); +void +testParamErrors() +{ + testcase("Test parameter errors"); - { - // No params - auto resp = env.rpc("json", "simulate"); - BEAST_EXPECT(resp[jss::error_message] == "Syntax error."); - } - { - // Providing both `tx_json` and `tx_blob` - Json::Value params = Json::objectValue; - params[jss::tx_json] = Json::objectValue; - params[jss::tx_blob] = "1200"; + using namespace jtx; + Env env(*this); + Account const alice("alice"); - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == "Invalid parameters."); - } - { - // `binary` isn't a boolean - Json::Value params = Json::objectValue; - params[jss::tx_blob] = "1200"; - params[jss::binary] = "100"; - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == - "Invalid field 'binary'."); - } - { - // Empty `tx_json` - Json::Value params = Json::objectValue; - params[jss::tx_json] = Json::objectValue; + { + // No params + auto resp = env.rpc("json", "simulate"); + BEAST_EXPECT(resp[jss::error_message] == "Syntax error."); + } + { + // Providing both `tx_json` and `tx_blob` + Json::Value params = Json::objectValue; + params[jss::tx_json] = Json::objectValue; + params[jss::tx_blob] = "1200"; - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == - "Missing field 'tx.TransactionType'."); - } - { - // No tx.account - Json::Value params = Json::objectValue; - Json::Value tx_json = Json::objectValue; - tx_json[jss::TransactionType] = jss::Payment; - params[jss::tx_json] = tx_json; + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == "Invalid parameters."); + } + { + // `binary` isn't a boolean + Json::Value params = Json::objectValue; + params[jss::tx_blob] = "1200"; + params[jss::binary] = "100"; + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == "Invalid field 'binary'."); + } + { + // Empty `tx_json` + Json::Value params = Json::objectValue; + params[jss::tx_json] = Json::objectValue; - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == - "Missing field 'tx.Account'."); - } - { - // Empty `tx_blob` - Json::Value params = Json::objectValue; - params[jss::tx_blob] = ""; + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Missing field 'tx.TransactionType'."); + } + { + // No tx.account + Json::Value params = Json::objectValue; + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::Payment; + params[jss::tx_json] = tx_json; - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == - "Invalid field 'tx_blob'."); - } - { - // Non-string `tx_blob` - Json::Value params; - params[jss::tx_blob] = 1.1; + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Missing field 'tx.Account'."); + } + { + // Empty `tx_blob` + Json::Value params = Json::objectValue; + params[jss::tx_blob] = ""; - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == - "Invalid field 'tx_blob'."); - } - { - // Non-object `tx_json` - Json::Value params = Json::objectValue; - params[jss::tx_json] = ""; + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Invalid field 'tx_blob'."); + } + { + // Non-string `tx_blob` + Json::Value params; + params[jss::tx_blob] = 1.1; - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == - "Invalid field 'tx_json', not object."); - } - { - // Invalid transaction - Json::Value params = Json::objectValue; - Json::Value tx_json = Json::objectValue; - tx_json[jss::TransactionType] = jss::Payment; - tx_json[jss::Account] = env.master.human(); - params[jss::tx_json] = tx_json; - - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_exception] == - "Field 'Destination' is required but missing."); - } - { - // Bad account - Json::Value params; - Json::Value tx_json = Json::objectValue; - tx_json[jss::TransactionType] = jss::AccountSet; - tx_json[jss::Account] = "badAccount"; - params[jss::tx_json] = tx_json; - - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT(resp[jss::result][jss::error] == "srcActMalformed"); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == - "Invalid field 'tx.Account'."); - } - { - // Account doesn't exist for Sequence autofill - Json::Value params; - Json::Value tx_json = Json::objectValue; - tx_json[jss::TransactionType] = jss::AccountSet; - tx_json[jss::Account] = alice.human(); - params[jss::tx_json] = tx_json; - - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == - "Source account not found."); - } - { - // Invalid transaction - Json::Value params; - Json::Value tx_json = Json::objectValue; - tx_json[jss::TransactionType] = jss::AccountSet; - tx_json[jss::Account] = alice.human(); - tx_json[jss::tx_json] = jss::AccountSet; - params[jss::tx_json] = tx_json; - - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == - "Source account not found."); - } + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Invalid field 'tx_blob'."); + } + { + // Non-object `tx_json` + Json::Value params = Json::objectValue; + params[jss::tx_json] = ""; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Invalid field 'tx_json', not object."); + } + { + // Invalid transaction + Json::Value params = Json::objectValue; + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::Payment; + tx_json[jss::Account] = env.master.human(); + params[jss::tx_json] = tx_json; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_exception] == + "Field 'Destination' is required but missing."); + } + { + // Bad account + Json::Value params; + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::AccountSet; + tx_json[jss::Account] = "badAccount"; + params[jss::tx_json] = tx_json; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT(resp[jss::result][jss::error] == "srcActMalformed"); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Invalid field 'tx.Account'."); + } + { + // Account doesn't exist for Sequence autofill + Json::Value params; + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::AccountSet; + tx_json[jss::Account] = alice.human(); + params[jss::tx_json] = tx_json; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Source account not found."); } - void - testSuccessfulTransaction() { - testcase("Successful transaction"); + // Invalid transaction + Json::Value params; + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::AccountSet; + tx_json[jss::Account] = alice.human(); + tx_json[jss::tx_json] = jss::AccountSet; + params[jss::tx_json] = tx_json; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Source account not found."); + } +} +void +testSuccessfulTransaction() +{ + testcase("Successful transaction"); - using namespace jtx; - Env env(*this); - static auto const newDomain = "123ABC"; + using namespace jtx; + Env env(*this); + static auto const newDomain = "123ABC"; - { - auto testSimulation = [&](Json::Value& params, Json::Value& tx) { - auto resp = env.rpc("json", "simulate", to_string(params)); - auto result = resp[jss::result]; - checkBasicReturnValidity(result, tx); + { + auto validateOutput = [&](Json::Value resp, Json::Value& tx) { + auto result = resp[jss::result]; + checkBasicReturnValidity(result, tx); - BEAST_EXPECT(result[jss::engine_result] == "tesSUCCESS"); - BEAST_EXPECT(result[jss::engine_result_code] == 0); - BEAST_EXPECT( - result[jss::engine_result_message] == - "The simulated transaction would have been applied."); + BEAST_EXPECT(result[jss::engine_result] == "tesSUCCESS"); + BEAST_EXPECT(result[jss::engine_result_code] == 0); + BEAST_EXPECT( + result[jss::engine_result_message] == + "The simulated transaction would have been applied."); - if (BEAST_EXPECT(result.isMember(jss::metadata))) + if (BEAST_EXPECT(result.isMember(jss::metadata))) + { + Json::Value metadata; + if (result[jss::metadata].isString()) { - auto metadata = result[jss::metadata]; - if (BEAST_EXPECT( - metadata.isMember(sfAffectedNodes.jsonName))) - { - BEAST_EXPECT( - metadata[sfAffectedNodes.jsonName].size() == 1); - auto node = metadata[sfAffectedNodes.jsonName][0u]; - if (BEAST_EXPECT( - node.isMember(sfModifiedNode.jsonName))) - { - auto modifiedNode = node[sfModifiedNode.jsonName]; - BEAST_EXPECT( - modifiedNode[sfLedgerEntryType.jsonName] == - "AccountRoot"); - auto finalFields = - modifiedNode[sfFinalFields.jsonName]; - BEAST_EXPECT( - finalFields[sfDomain.jsonName] == newDomain); - } - } - BEAST_EXPECT(metadata[sfTransactionIndex.jsonName] == 0); - BEAST_EXPECT( - metadata[sfTransactionResult.jsonName] == "tesSUCCESS"); + auto unHexed = strUnHex(result[jss::metadata].asString()); + SerialIter sitTrans(makeSlice(*unHexed)); + metadata = STObject(std::ref(sitTrans), sfGeneric) + .getJson(JsonOptions::none); } - }; - - auto testTx = [&](Json::Value& tx) { + else { - Json::Value params; - params[jss::tx_json] = tx; - testSimulation(params, tx); + metadata = result[jss::metadata]; } + if (BEAST_EXPECT(metadata.isMember(sfAffectedNodes.jsonName))) { - STParsedJSONObject parsed(std::string(jss::tx_json), tx); - if (BEAST_EXPECT(parsed.object.has_value())) + BEAST_EXPECT( + metadata[sfAffectedNodes.jsonName].size() == 1); + auto node = metadata[sfAffectedNodes.jsonName][0u]; + if (BEAST_EXPECT(node.isMember(sfModifiedNode.jsonName))) { - Json::Value params; - params[jss::tx_blob] = - strHex(parsed.object->getSerializer().peekData()); - testSimulation(params, tx); + auto modifiedNode = node[sfModifiedNode.jsonName]; + BEAST_EXPECT( + modifiedNode[sfLedgerEntryType.jsonName] == + "AccountRoot"); + auto finalFields = modifiedNode[sfFinalFields.jsonName]; + BEAST_EXPECT( + finalFields[sfDomain.jsonName] == newDomain); } } - }; + BEAST_EXPECT(metadata[sfTransactionIndex.jsonName] == 0); + BEAST_EXPECT( + metadata[sfTransactionResult.jsonName] == "tesSUCCESS"); + } + }; - Json::Value tx; + Json::Value tx; - tx[jss::Account] = Account::master.human(); - tx[jss::TransactionType] = jss::AccountSet; - tx[sfDomain.jsonName] = newDomain; + tx[jss::Account] = Account::master.human(); + tx[jss::TransactionType] = jss::AccountSet; + tx[sfDomain.jsonName] = newDomain; - // test with autofill - testTx(tx); + // test with autofill + testTx(env, tx, validateOutput); - tx[sfSigningPubKey.jsonName] = ""; - tx[sfTxnSignature.jsonName] = ""; - tx[sfSequence.jsonName] = 1; - tx[sfFee.jsonName] = "12"; + tx[sfSigningPubKey.jsonName] = ""; + tx[sfTxnSignature.jsonName] = ""; + tx[sfSequence.jsonName] = 1; + tx[sfFee.jsonName] = "12"; - // test without autofill - testTx(tx); + // test without autofill + testTx(env, tx, validateOutput); - // TODO: check that the ledger wasn't affected - } + // TODO: check that the ledger wasn't affected } +} - void - testTransactionNonTecFailure() - { - testcase("Transaction non-tec failure"); +void +testTransactionNonTecFailure() +{ + testcase("Transaction non-tec failure"); - using namespace jtx; - Env env(*this); - Account const alice("alice"); + using namespace jtx; + Env env(*this); + Account const alice("alice"); - { - auto testSimulation = [&](Json::Value& params, Json::Value& tx) { - auto resp = env.rpc("json", "simulate", to_string(params)); + { + std::function testSimulation = + [&](Json::Value resp, Json::Value& tx) { auto result = resp[jss::result]; checkBasicReturnValidity(result, tx); @@ -322,60 +339,40 @@ class Simulate_test : public beast::unit_test::suite BEAST_EXPECT(!result.isMember(jss::metadata)); }; - auto testTx = [&](Json::Value& tx) { - { - Json::Value params; - params[jss::tx_json] = tx; - testSimulation(params, tx); - params[jss::binary] = true; - testSimulation(params, tx); - } - { - STParsedJSONObject parsed(std::string(jss::tx_json), tx); - if (BEAST_EXPECT(parsed.object.has_value())) - { - Json::Value params; - params[jss::tx_blob] = - strHex(parsed.object->getSerializer().peekData()); - testSimulation(params, tx); - } - } - }; - - Json::Value tx; + Json::Value tx; - tx[jss::Account] = Account::master.human(); - tx[jss::TransactionType] = jss::Payment; - tx[sfDestination.jsonName] = alice.human(); - tx[sfAmount.jsonName] = "0"; // invalid amount + tx[jss::Account] = Account::master.human(); + tx[jss::TransactionType] = jss::Payment; + tx[sfDestination.jsonName] = alice.human(); + tx[sfAmount.jsonName] = "0"; // invalid amount - // test with autofill - testTx(tx); + // test with autofill + testTx(env, tx, testSimulation); - tx[sfSigningPubKey.jsonName] = ""; - tx[sfTxnSignature.jsonName] = ""; - tx[sfSequence.jsonName] = 1; - tx[sfFee.jsonName] = "12"; + tx[sfSigningPubKey.jsonName] = ""; + tx[sfTxnSignature.jsonName] = ""; + tx[sfSequence.jsonName] = 1; + tx[sfFee.jsonName] = "12"; - // test without autofill - testTx(tx); + // test without autofill + testTx(env, tx, testSimulation); - // TODO: check that the ledger wasn't affected - } + // TODO: check that the ledger wasn't affected } +} - void - testTransactionTecFailure() - { - testcase("Transaction tec failure"); +void +testTransactionTecFailure() +{ + testcase("Transaction tec failure"); - using namespace jtx; - Env env(*this); - Account const alice("alice"); + using namespace jtx; + Env env(*this); + Account const alice("alice"); - { - auto testSimulation = [&](Json::Value& params, Json::Value& tx) { - auto resp = env.rpc("json", "simulate", to_string(params)); + { + std::function testSimulation = + [&](Json::Value resp, Json::Value& tx) { auto result = resp[jss::result]; checkBasicReturnValidity(result, tx); @@ -390,7 +387,7 @@ class Simulate_test : public beast::unit_test::suite if (BEAST_EXPECT(result.isMember(jss::metadata))) { Json::Value metadata; - if (params.get(jss::binary, false).asBool()) + if (result[jss::metadata].isString()) { auto unHexed = strUnHex(result[jss::metadata].asString()); @@ -429,58 +426,38 @@ class Simulate_test : public beast::unit_test::suite } }; - auto testTx = [&](Json::Value& tx) { - { - Json::Value params; - params[jss::tx_json] = tx; - testSimulation(params, tx); - params[jss::binary] = true; - testSimulation(params, tx); - } - { - STParsedJSONObject parsed(std::string(jss::tx_json), tx); - if (BEAST_EXPECT(parsed.object.has_value())) - { - Json::Value params; - params[jss::tx_blob] = - strHex(parsed.object->getSerializer().peekData()); - testSimulation(params, tx); - } - } - }; + Json::Value tx; - Json::Value tx; + tx[jss::Account] = Account::master.human(); + tx[jss::TransactionType] = jss::Payment; + tx[sfDestination.jsonName] = alice.human(); + tx[sfAmount.jsonName] = "1"; // not enough to create an account - tx[jss::Account] = Account::master.human(); - tx[jss::TransactionType] = jss::Payment; - tx[sfDestination.jsonName] = alice.human(); - tx[sfAmount.jsonName] = "1"; // not enough to create an account + // test with autofill + testTx(env, tx, testSimulation); - // test with autofill - testTx(tx); + tx[sfSigningPubKey.jsonName] = ""; + tx[sfTxnSignature.jsonName] = ""; + tx[sfSequence.jsonName] = 1; + tx[sfFee.jsonName] = "10"; - tx[sfSigningPubKey.jsonName] = ""; - tx[sfTxnSignature.jsonName] = ""; - tx[sfSequence.jsonName] = 1; - tx[sfFee.jsonName] = "10"; + // test without autofill + testTx(env, tx, testSimulation); - // test without autofill - testTx(tx); - - // TODO: check that the ledger wasn't affected - } + // TODO: check that the ledger wasn't affected } +} public: - void - run() override - { - testParamErrors(); - testSuccessfulTransaction(); - testTransactionNonTecFailure(); - testTransactionTecFailure(); - } -}; +void +run() override +{ + testParamErrors(); + testSuccessfulTransaction(); + testTransactionNonTecFailure(); + testTransactionTecFailure(); +} +}; // namespace ripple BEAST_DEFINE_TESTSUITE(Simulate, rpc, ripple); From cc011c06d47a20e7519efc00bfe426d531d70252 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 18 Jul 2024 13:48:01 -0700 Subject: [PATCH 23/42] improve codecov --- src/test/rpc/Simulate_test.cpp | 12 +++++++----- src/xrpld/app/tx/detail/Transactor.cpp | 4 ++-- src/xrpld/rpc/handlers/Simulate.cpp | 3 ++- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp index 16a5812b02a..49d2f624f27 100644 --- a/src/test/rpc/Simulate_test.cpp +++ b/src/test/rpc/Simulate_test.cpp @@ -109,8 +109,10 @@ testParamErrors() { // No params - auto resp = env.rpc("json", "simulate"); - BEAST_EXPECT(resp[jss::error_message] == "Syntax error."); + Json::Value params = Json::objectValue; + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == "Invalid parameters."); } { // Providing both `tx_json` and `tx_blob` @@ -228,14 +230,14 @@ testParamErrors() Json::Value params; Json::Value tx_json = Json::objectValue; tx_json[jss::TransactionType] = jss::AccountSet; - tx_json[jss::Account] = alice.human(); - tx_json[jss::tx_json] = jss::AccountSet; + tx_json[jss::Account] = env.master.human(); + tx_json["foo"] = "bar"; params[jss::tx_json] = tx_json; auto resp = env.rpc("json", "simulate", to_string(params)); BEAST_EXPECT( resp[jss::result][jss::error_message] == - "Source account not found."); + "Field 'tx_json.foo' is unknown."); } } void diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index ec3f4a7d2f2..7d115d0f381 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -138,7 +138,7 @@ preflight2(PreflightContext const& ctx) return tesSUCCESS; // NOTE: This code should never be hit because it's checked in the // `simulate` RPC - return temINVALID; + return temINVALID; // LCOV_EXCL_LINE } auto const sigValid = checkValidity( ctx.app.getHashRouter(), ctx.tx, ctx.rules, ctx.app.config()); @@ -493,7 +493,7 @@ Transactor::checkSign(PreclaimContext const& ctx) if (ctx.tx.getSigningPubKey().empty() && ctx.tx.getSignature().empty()) return tesSUCCESS; // Already checked in preflight2, should never be hit - return temINVALID; + return temINVALID; // LCOV_EXCL_LINE } // If the pk is empty, then we must be multi-signing. if (ctx.tx.getSigningPubKey().empty()) diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index 80dcf625189..e4def9425b2 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -260,10 +260,11 @@ doSimulate(RPC::JsonContext& context) } catch (std::exception& e) { + // LCOV_EXCL_START this is just in case, so rippled doesn't crash jvResult[jss::error] = "internalSimulate"; jvResult[jss::error_exception] = e.what(); - return jvResult; + // LCOV_EXCL_STOP } } From db7ad89d268fc871513e767db6388d7a7cac10e9 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 18 Jul 2024 14:16:27 -0700 Subject: [PATCH 24/42] more codecov tests --- src/test/rpc/Simulate_test.cpp | 15 +++++++++++++++ src/xrpld/rpc/handlers/Simulate.cpp | 4 ++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp index 49d2f624f27..7cda1e30554 100644 --- a/src/test/rpc/Simulate_test.cpp +++ b/src/test/rpc/Simulate_test.cpp @@ -239,6 +239,21 @@ testParamErrors() resp[jss::result][jss::error_message] == "Field 'tx_json.foo' is unknown."); } + { + // non-string first param for CLI + auto resp = env.rpc("simulate", 1); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == "Invalid parameters."); + } + { + // non-string second param for CLI + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::AccountSet; + tx_json[jss::Account] = alice.human(); + auto resp = env.rpc("simulate", to_string(tx_json), 1); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == "Invalid parameters."); + } } void testSuccessfulTransaction() diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index e4def9425b2..a32e71b08c2 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -74,7 +74,7 @@ autofillTx(Json::Value& tx_json, RPC::JsonContext& context) if (!accountStr.isString()) { // sanity check, should fail earlier - return RPC::invalid_field_error("tx.Account"); + return RPC::invalid_field_error("tx.Account"); // LCOV_EXCL_LINE } auto const srcAddressID = parseBase58(tx_json[jss::Account].asString()); @@ -258,9 +258,9 @@ doSimulate(RPC::JsonContext& context) return jvResult; } + // LCOV_EXCL_START this is just in case, so rippled doesn't crash catch (std::exception& e) { - // LCOV_EXCL_START this is just in case, so rippled doesn't crash jvResult[jss::error] = "internalSimulate"; jvResult[jss::error_exception] = e.what(); return jvResult; From b30b3b98d4cdf01b35f95062cf88abcc724d0763 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 18 Jul 2024 15:32:57 -0700 Subject: [PATCH 25/42] fix tests --- src/test/rpc/Simulate_test.cpp | 11 ++--------- src/xrpld/net/detail/RPCCall.cpp | 2 -- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp index 7cda1e30554..362594915dd 100644 --- a/src/test/rpc/Simulate_test.cpp +++ b/src/test/rpc/Simulate_test.cpp @@ -239,20 +239,13 @@ testParamErrors() resp[jss::result][jss::error_message] == "Field 'tx_json.foo' is unknown."); } - { - // non-string first param for CLI - auto resp = env.rpc("simulate", 1); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == "Invalid parameters."); - } { // non-string second param for CLI Json::Value tx_json = Json::objectValue; tx_json[jss::TransactionType] = jss::AccountSet; tx_json[jss::Account] = alice.human(); - auto resp = env.rpc("simulate", to_string(tx_json), 1); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == "Invalid parameters."); + auto resp = env.rpc("simulate", to_string(tx_json), "1"); + BEAST_EXPECT(resp[jss::error_message] == "Invalid parameters."); } } void diff --git a/src/xrpld/net/detail/RPCCall.cpp b/src/xrpld/net/detail/RPCCall.cpp index 15fa38be0c9..c07bf8e0d07 100644 --- a/src/xrpld/net/detail/RPCCall.cpp +++ b/src/xrpld/net/detail/RPCCall.cpp @@ -935,8 +935,6 @@ class RPCParser Json::Reader reader; Json::Value jvRequest{Json::objectValue}; - if (!jvParams[0u].isString()) - return rpcError(rpcINVALID_PARAMS); if (reader.parse(jvParams[0u].asString(), txJSON)) { jvRequest[jss::tx_json] = txJSON; From f222f442ae27022e6eceece1e2276c4180fa1986 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Fri, 19 Jul 2024 14:43:39 -0700 Subject: [PATCH 26/42] add public key support --- src/xrpld/app/tx/detail/Transactor.cpp | 12 +++++------- src/xrpld/rpc/handlers/Simulate.cpp | 23 ++++++++++++++++++++--- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index 7d115d0f381..7fd82eecd37 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -132,9 +132,9 @@ preflight1(PreflightContext const& ctx) NotTEC preflight2(PreflightContext const& ctx) { - if (ctx.flags & tapDRY_RUN) + if (ctx.flags & tapDRY_RUN) // simulation { - if (ctx.tx.getSigningPubKey().empty() && ctx.tx.getSignature().empty()) + if (ctx.tx.getSignature().empty()) return tesSUCCESS; // NOTE: This code should never be hit because it's checked in the // `simulate` RPC @@ -488,12 +488,10 @@ Transactor::apply() NotTEC Transactor::checkSign(PreclaimContext const& ctx) { - if (ctx.flags & tapDRY_RUN) + if (ctx.flags & tapDRY_RUN && ctx.tx.getSigningPubKey().empty() && + ctx.tx.getFieldArray(sfSigners).empty()) { - if (ctx.tx.getSigningPubKey().empty() && ctx.tx.getSignature().empty()) - return tesSUCCESS; - // Already checked in preflight2, should never be hit - return temINVALID; // LCOV_EXCL_LINE + return tesSUCCESS; } // If the pk is empty, then we must be multi-signing. if (ctx.tx.getSigningPubKey().empty()) diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index a32e71b08c2..158f3f39a43 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -51,10 +51,27 @@ autofillTx(Json::Value& tx_json, RPC::JsonContext& context) // autofill SigningPubKey tx_json[sfSigningPubKey.jsonName] = ""; } - else if (tx_json[sfSigningPubKey.jsonName] != "") + if (tx_json.isMember(sfSigners.jsonName)) { - // Transaction must not be signed - return rpcError(rpcTX_SIGNED); + if (!tx_json[sfSigners.jsonName].isArray()) + return RPC::invalid_field_error("tx.Signers"); + // check multisigned signers + for (auto& signer : tx_json[sfSigners.jsonName]) + { + if (!signer.isMember(sfSigner.jsonName) || + !signer[sfSigner.jsonName].isObject()) + return RPC::invalid_field_error("tx.Signers"); + if (!signer[sfSigner.jsonName].isMember(sfTxnSignature.jsonName)) + { + // autofill TxnSignature + signer[sfSigner.jsonName][sfTxnSignature.jsonName] = ""; + } + else if (signer[sfSigner.jsonName][sfTxnSignature.jsonName] != "") + { + // Transaction must not be signed + return rpcError(rpcTX_SIGNED); + } + } } if (!tx_json.isMember(sfTxnSignature.jsonName)) { From a3c0bb4595ee9170eca3d6cf7e30478f9dae046a Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Mon, 22 Jul 2024 10:19:05 -0400 Subject: [PATCH 27/42] update field names --- src/test/rpc/Simulate_test.cpp | 28 +++++++++++++++++----------- src/xrpld/rpc/handlers/Simulate.cpp | 4 ++-- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp index 362594915dd..56fb0d68d20 100644 --- a/src/test/rpc/Simulate_test.cpp +++ b/src/test/rpc/Simulate_test.cpp @@ -144,7 +144,7 @@ testParamErrors() "Missing field 'tx.TransactionType'."); } { - // No tx.account + // No tx.Account Json::Value params = Json::objectValue; Json::Value tx_json = Json::objectValue; tx_json[jss::TransactionType] = jss::Payment; @@ -240,7 +240,7 @@ testParamErrors() "Field 'tx_json.foo' is unknown."); } { - // non-string second param for CLI + // non-`"binary"` second param for CLI Json::Value tx_json = Json::objectValue; tx_json[jss::TransactionType] = jss::AccountSet; tx_json[jss::Account] = alice.human(); @@ -268,19 +268,21 @@ testSuccessfulTransaction() result[jss::engine_result_message] == "The simulated transaction would have been applied."); - if (BEAST_EXPECT(result.isMember(jss::metadata))) + if (BEAST_EXPECT( + result.isMember(jss::meta) || + result.isMember(jss::meta_blob))) { Json::Value metadata; - if (result[jss::metadata].isString()) + if (result.isMember(jss::meta_blob)) { - auto unHexed = strUnHex(result[jss::metadata].asString()); + auto unHexed = strUnHex(result[jss::meta_blob].asString()); SerialIter sitTrans(makeSlice(*unHexed)); metadata = STObject(std::ref(sitTrans), sfGeneric) .getJson(JsonOptions::none); } else { - metadata = result[jss::metadata]; + metadata = result[jss::meta]; } if (BEAST_EXPECT(metadata.isMember(sfAffectedNodes.jsonName))) { @@ -346,7 +348,9 @@ testTransactionNonTecFailure() result[jss::engine_result_message] == "Can only send positive amounts."); - BEAST_EXPECT(!result.isMember(jss::metadata)); + BEAST_EXPECT( + !result.isMember(jss::meta) && + !result.isMember(jss::meta_blob)); }; Json::Value tx; @@ -394,20 +398,22 @@ testTransactionTecFailure() "Destination does not exist. Too little XRP sent to create " "it."); - if (BEAST_EXPECT(result.isMember(jss::metadata))) + if (BEAST_EXPECT( + result.isMember(jss::meta) || + result.isMember(jss::meta_blob))) { Json::Value metadata; - if (result[jss::metadata].isString()) + if (result.isMember(jss::meta_blob)) { auto unHexed = - strUnHex(result[jss::metadata].asString()); + strUnHex(result[jss::meta_blob].asString()); SerialIter sitTrans(makeSlice(*unHexed)); metadata = STObject(std::ref(sitTrans), sfGeneric) .getJson(JsonOptions::none); } else { - metadata = result[jss::metadata]; + metadata = result[jss::meta]; } if (BEAST_EXPECT( metadata.isMember(sfAffectedNodes.jsonName))) diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index 158f3f39a43..41de75fec14 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -255,11 +255,11 @@ doSimulate(RPC::JsonContext& context) { auto const metaBlob = result.metadata->getAsObject().getSerializer().getData(); - jvResult[jss::metadata] = strHex(makeSlice(metaBlob)); + jvResult[jss::meta_blob] = strHex(makeSlice(metaBlob)); } else { - jvResult[jss::metadata] = + jvResult[jss::meta] = result.metadata->getJson(JsonOptions::none); } } From 6c6ecc46873fc336bd75d5de65f9cc77ff567e10 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Mon, 22 Jul 2024 12:17:46 -0400 Subject: [PATCH 28/42] fix fee issues --- src/xrpld/rpc/detail/TransactionSign.cpp | 150 ++++++++++++++--------- src/xrpld/rpc/detail/TransactionSign.h | 1 + src/xrpld/rpc/handlers/Simulate.cpp | 26 ++-- 3 files changed, 112 insertions(+), 65 deletions(-) diff --git a/src/xrpld/rpc/detail/TransactionSign.cpp b/src/xrpld/rpc/detail/TransactionSign.cpp index 5f0295f9782..077d8bf801f 100644 --- a/src/xrpld/rpc/detail/TransactionSign.cpp +++ b/src/xrpld/rpc/detail/TransactionSign.cpp @@ -25,6 +25,7 @@ #include #include #include // Validity::Valid +#include #include #include #include @@ -445,6 +446,28 @@ transactionPreProcessImpl( return rpcError(rpcSRC_ACT_NOT_FOUND); } + if (signingArgs.editFields()) + { + if (!tx_json.isMember(jss::Sequence)) + { + bool const hasTicketSeq = + tx_json.isMember(sfTicketSequence.jsonName); + if (!hasTicketSeq && !sle) + { + JLOG(j.debug()) + << "transactionSign: Failed to find source account " + << "in current ledger: " << toBase58(srcAddressID); + + return rpcError(rpcSRC_ACT_NOT_FOUND); + } + tx_json[jss::Sequence] = + hasTicketSeq ? 0 : app.getTxQ().nextQueuableSeq(sle).value(); + } + + if (!tx_json.isMember(jss::Flags)) + tx_json[jss::Flags] = tfFullyCanonicalSig; + } + { Json::Value err = checkFee( params, @@ -470,28 +493,6 @@ transactionPreProcessImpl( return err; } - if (signingArgs.editFields()) - { - if (!tx_json.isMember(jss::Sequence)) - { - bool const hasTicketSeq = - tx_json.isMember(sfTicketSequence.jsonName); - if (!hasTicketSeq && !sle) - { - JLOG(j.debug()) - << "transactionSign: Failed to find source account " - << "in current ledger: " << toBase58(srcAddressID); - - return rpcError(rpcSRC_ACT_NOT_FOUND); - } - tx_json[jss::Sequence] = - hasTicketSeq ? 0 : app.getTxQ().nextQueuableSeq(sle).value(); - } - - if (!tx_json.isMember(jss::Flags)) - tx_json[jss::Flags] = tfFullyCanonicalSig; - } - // If multisigning there should not be a single signature and vice versa. if (signingArgs.isMultiSigning()) { @@ -707,6 +708,68 @@ transactionFormatResultImpl(Transaction::pointer tpTrans, unsigned apiVersion) //------------------------------------------------------------------------------ +Expected +getBaseFee(Application const& app, Json::Value tx) +{ + if (!tx.isMember(jss::Fee)) + { + tx[jss::Fee] = "0"; + } + if (!tx.isMember(jss::Sequence)) + { + tx[jss::Sequence] = "0"; + } + if (!tx.isMember(jss::SigningPubKey)) + { + tx[jss::SigningPubKey] = ""; + } + if (!tx.isMember(jss::TxnSignature)) + { + tx[jss::TxnSignature] = ""; + } + if (tx.isMember(sfSigners.jsonName)) + { + if (!tx[sfSigners.jsonName].isArray()) + Throw(RPC::invalid_field_message("tx.Signers")); + // check multisigned signers + for (auto& signer : tx[sfSigners.jsonName]) + { + if (!signer.isMember(sfSigner.jsonName) || + !signer[sfSigner.jsonName].isObject()) + Throw( + RPC::invalid_field_message("tx.Signers")); + if (!signer[sfSigner.jsonName].isMember(sfTxnSignature.jsonName)) + { + // autofill TxnSignature + signer[sfSigner.jsonName][sfTxnSignature.jsonName] = ""; + } + } + } + STParsedJSONObject parsed(std::string(jss::tx_json), tx); + if (!parsed.object.has_value()) + { + Json::Value error; + error[jss::error] = parsed.error[jss::error]; + error[jss::error_code] = parsed.error[jss::error_code]; + error[jss::error_message] = parsed.error[jss::error_message]; + return Unexpected(error); + } + + try + { + STTx const& stTx = STTx(std::move(parsed.object.value())); + return calculateBaseFee(*app.openLedger().current(), stTx); + } + catch (std::exception& e) + { + Json::Value error; + error[jss::error] = "invalidTransaction"; + error[jss::error_exception] = e.what(); + + return Unexpected(error); + } +} + Json::Value getCurrentFee( Role const role, @@ -714,10 +777,14 @@ getCurrentFee( LoadFeeTrack const& feeTrack, TxQ const& txQ, Application const& app, + Json::Value tx, int mult, int div) { - XRPAmount const feeDefault = config.FEES.reference_fee; + auto const baseFeeExpected = getBaseFee(app, tx); + if (!baseFeeExpected) + return baseFeeExpected.error(); + XRPAmount const feeDefault = *baseFeeExpected; auto ledger = app.openLedger().current(); // Administrative and identified endpoints are exempt from local fees. @@ -809,38 +876,11 @@ checkFee( } } - XRPAmount const feeDefault = config.FEES.reference_fee; - - auto ledger = app.openLedger().current(); - // Administrative and identified endpoints are exempt from local fees. - XRPAmount const loadFee = - scaleFeeLoad(feeDefault, feeTrack, ledger->fees(), isUnlimited(role)); - XRPAmount fee = loadFee; - { - auto const metrics = txQ.getMetrics(*ledger); - auto const baseFee = ledger->fees().base; - auto escalatedFee = - toDrops(metrics.openLedgerFeeLevel - FeeLevel64(1), baseFee) + 1; - fee = std::max(fee, escalatedFee); - } - - auto const limit = [&]() { - // Scale fee units to drops: - auto const result = mulDiv(feeDefault, mult, div); - if (!result) - Throw("mulDiv"); - return *result; - }(); - - if (fee > limit) - { - std::stringstream ss; - ss << "Fee of " << fee << " exceeds the requested tx limit of " - << limit; - return RPC::make_error(rpcHIGH_FEE, ss.str()); - } - - tx[jss::Fee] = getCurrentFee(role, config, feeTrack, txQ, app, mult, div); + auto feeOrError = + getCurrentFee(role, config, feeTrack, txQ, app, tx, mult, div); + if (feeOrError.isMember(jss::error)) + return feeOrError; + tx[jss::Fee] = feeOrError; return Json::Value(); } diff --git a/src/xrpld/rpc/detail/TransactionSign.h b/src/xrpld/rpc/detail/TransactionSign.h index 43077264d74..a89e775189e 100644 --- a/src/xrpld/rpc/detail/TransactionSign.h +++ b/src/xrpld/rpc/detail/TransactionSign.h @@ -43,6 +43,7 @@ getCurrentFee( LoadFeeTrack const& feeTrack, TxQ const& txQ, Application const& app, + Json::Value tx, int mult = Tuning::defaultAutoFillFeeMultiplier, int div = Tuning::defaultAutoFillFeeDivisor); diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index 41de75fec14..8b042f309d6 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -36,16 +36,6 @@ namespace ripple { std::optional autofillTx(Json::Value& tx_json, RPC::JsonContext& context) { - if (!tx_json.isMember(jss::Fee)) - { - // autofill Fee - tx_json[jss::Fee] = RPC::getCurrentFee( - context.role, - context.app.config(), - context.app.getFeeTrack(), - context.app.getTxQ(), - context.app); - } if (!tx_json.isMember(sfSigningPubKey.jsonName)) { // autofill SigningPubKey @@ -115,6 +105,22 @@ autofillTx(Json::Value& tx_json, RPC::JsonContext& context) ? 0 : context.app.getTxQ().nextQueuableSeq(sle).value(); } + if (!tx_json.isMember(jss::Fee)) + { + // autofill Fee + // Must happen after all the other autofills happen + // Error handling/messaging works better that way + auto feeOrError = RPC::getCurrentFee( + context.role, + context.app.config(), + context.app.getFeeTrack(), + context.app.getTxQ(), + context.app, + tx_json); + if (feeOrError.isMember(jss::error)) + return feeOrError; + tx_json[jss::Fee] = feeOrError; + } return std::nullopt; } From c8f3862a448343e723ad6e78de84f904b70c6763 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Mon, 22 Jul 2024 13:32:12 -0400 Subject: [PATCH 29/42] backwards compatibility --- src/xrpld/rpc/detail/TransactionSign.cpp | 43 +++++++++--------------- 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/src/xrpld/rpc/detail/TransactionSign.cpp b/src/xrpld/rpc/detail/TransactionSign.cpp index 077d8bf801f..bbd1ddc64b7 100644 --- a/src/xrpld/rpc/detail/TransactionSign.cpp +++ b/src/xrpld/rpc/detail/TransactionSign.cpp @@ -469,25 +469,25 @@ transactionPreProcessImpl( } { - Json::Value err = checkFee( + Json::Value err = checkPayment( params, + tx_json, + srcAddressID, role, - verify && signingArgs.editFields(), - app.config(), - app.getFeeTrack(), - app.getTxQ(), - app); + app, + verify && signingArgs.editFields()); if (RPC::contains_error(err)) return err; - err = checkPayment( + err = checkFee( params, - tx_json, - srcAddressID, role, - app, - verify && signingArgs.editFields()); + verify && signingArgs.editFields(), + app.config(), + app.getFeeTrack(), + app.getTxQ(), + app); if (RPC::contains_error(err)) return err; @@ -708,8 +708,8 @@ transactionFormatResultImpl(Transaction::pointer tpTrans, unsigned apiVersion) //------------------------------------------------------------------------------ -Expected -getBaseFee(Application const& app, Json::Value tx) +XRPAmount +getBaseFee(Application const& app, Config const& config, Json::Value tx) { if (!tx.isMember(jss::Fee)) { @@ -748,11 +748,7 @@ getBaseFee(Application const& app, Json::Value tx) STParsedJSONObject parsed(std::string(jss::tx_json), tx); if (!parsed.object.has_value()) { - Json::Value error; - error[jss::error] = parsed.error[jss::error]; - error[jss::error_code] = parsed.error[jss::error_code]; - error[jss::error_message] = parsed.error[jss::error_message]; - return Unexpected(error); + return config.FEES.reference_fee; } try @@ -762,11 +758,7 @@ getBaseFee(Application const& app, Json::Value tx) } catch (std::exception& e) { - Json::Value error; - error[jss::error] = "invalidTransaction"; - error[jss::error_exception] = e.what(); - - return Unexpected(error); + return config.FEES.reference_fee; } } @@ -781,10 +773,7 @@ getCurrentFee( int mult, int div) { - auto const baseFeeExpected = getBaseFee(app, tx); - if (!baseFeeExpected) - return baseFeeExpected.error(); - XRPAmount const feeDefault = *baseFeeExpected; + XRPAmount const feeDefault = getBaseFee(app, config, tx); auto ledger = app.openLedger().current(); // Administrative and identified endpoints are exempt from local fees. From 03cae0d7e84813f53ac57aa7096c280aa2d6f22c Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 23 Jul 2024 12:27:25 -0400 Subject: [PATCH 30/42] add tests --- src/test/rpc/Simulate_test.cpp | 789 ++++++++++++++--------- src/xrpld/rpc/detail/TransactionSign.cpp | 5 +- src/xrpld/rpc/handlers/Simulate.cpp | 40 +- 3 files changed, 511 insertions(+), 323 deletions(-) diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp index 56fb0d68d20..1e857c90145 100644 --- a/src/test/rpc/Simulate_test.cpp +++ b/src/test/rpc/Simulate_test.cpp @@ -38,7 +38,11 @@ namespace test { class Simulate_test : public beast::unit_test::suite { void - checkBasicReturnValidity(Json::Value& result, Json::Value& tx) + checkBasicReturnValidity( + Json::Value& result, + Json::Value& tx, + std::string fee = "10", + int sequence = 1) { BEAST_EXPECT(result[jss::applied] == false); BEAST_EXPECT(result.isMember(jss::engine_result)); @@ -65,338 +69,512 @@ class Simulate_test : public beast::unit_test::suite tx_json[jss::SigningPubKey] == tx.get(jss::SigningPubKey, "")); BEAST_EXPECT( tx_json[jss::TxnSignature] == tx.get(jss::TxnSignature, "")); - BEAST_EXPECT(tx_json[jss::Fee] == tx.get(jss::Fee, "10")); - BEAST_EXPECT(tx_json[jss::Sequence] == tx.get(jss::Sequence, 1)); + BEAST_EXPECT(tx_json[jss::Fee] == tx.get(jss::Fee, fee)); + BEAST_EXPECT(tx_json[jss::Sequence] == tx.get(jss::Sequence, sequence)); } void testTx( jtx::Env& env, Json::Value& tx, - std::function validate){ - {Json::Value params; - params[jss::tx_json] = tx; - validate(env.rpc("json", "simulate", to_string(params)), tx); - params[jss::binary] = true; - validate(env.rpc("json", "simulate", to_string(params)), tx); - validate(env.rpc("simulate", to_string(tx)), tx); - validate(env.rpc("simulate", to_string(tx), "binary"), tx); -} -{ - STParsedJSONObject parsed(std::string(jss::tx_json), tx); - auto const tx_blob = strHex(parsed.object->getSerializer().peekData()); - if (BEAST_EXPECT(parsed.object.has_value())) + std::function validate, + bool testSerialized = true) { Json::Value params; - params[jss::tx_blob] = tx_blob; + params[jss::tx_json] = tx; validate(env.rpc("json", "simulate", to_string(params)), tx); params[jss::binary] = true; validate(env.rpc("json", "simulate", to_string(params)), tx); + validate(env.rpc("simulate", to_string(tx)), tx); + validate(env.rpc("simulate", to_string(tx), "binary"), tx); + if (testSerialized) + { + // This cannot be tested in the multisign autofill scenario + // It is technically not a valid STObject, so the following line + // will crash + STParsedJSONObject parsed(std::string(jss::tx_json), tx); + auto const tx_blob = + strHex(parsed.object->getSerializer().peekData()); + if (BEAST_EXPECT(parsed.object.has_value())) + { + Json::Value params; + params[jss::tx_blob] = tx_blob; + validate(env.rpc("json", "simulate", to_string(params)), tx); + params[jss::binary] = true; + validate(env.rpc("json", "simulate", to_string(params)), tx); + } + validate(env.rpc("simulate", tx_blob), tx); + validate(env.rpc("simulate", tx_blob, "binary"), tx); + } } - validate(env.rpc("simulate", tx_blob), tx); - validate(env.rpc("simulate", tx_blob, "binary"), tx); -} -}; // namespace test - -void -testParamErrors() -{ - testcase("Test parameter errors"); - - using namespace jtx; - Env env(*this); - Account const alice("alice"); + void + testParamErrors() { - // No params - Json::Value params = Json::objectValue; - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == "Invalid parameters."); - } - { - // Providing both `tx_json` and `tx_blob` - Json::Value params = Json::objectValue; - params[jss::tx_json] = Json::objectValue; - params[jss::tx_blob] = "1200"; + testcase("Test parameter errors"); - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == "Invalid parameters."); - } - { - // `binary` isn't a boolean - Json::Value params = Json::objectValue; - params[jss::tx_blob] = "1200"; - params[jss::binary] = "100"; - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == "Invalid field 'binary'."); - } - { - // Empty `tx_json` - Json::Value params = Json::objectValue; - params[jss::tx_json] = Json::objectValue; + using namespace jtx; + Env env(*this); + Account const alice("alice"); - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == - "Missing field 'tx.TransactionType'."); - } - { - // No tx.Account - Json::Value params = Json::objectValue; - Json::Value tx_json = Json::objectValue; - tx_json[jss::TransactionType] = jss::Payment; - params[jss::tx_json] = tx_json; + { + // No params + Json::Value params = Json::objectValue; + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == "Invalid parameters."); + } + { + // Providing both `tx_json` and `tx_blob` + Json::Value params = Json::objectValue; + params[jss::tx_json] = Json::objectValue; + params[jss::tx_blob] = "1200"; - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == - "Missing field 'tx.Account'."); - } - { - // Empty `tx_blob` - Json::Value params = Json::objectValue; - params[jss::tx_blob] = ""; + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == "Invalid parameters."); + } + { + // `binary` isn't a boolean + Json::Value params = Json::objectValue; + params[jss::tx_blob] = "1200"; + params[jss::binary] = "100"; + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Invalid field 'binary'."); + } + { + // Empty `tx_json` + Json::Value params = Json::objectValue; + params[jss::tx_json] = Json::objectValue; - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == - "Invalid field 'tx_blob'."); - } - { - // Non-string `tx_blob` - Json::Value params; - params[jss::tx_blob] = 1.1; + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Missing field 'tx.TransactionType'."); + } + { + // No tx.Account + Json::Value params = Json::objectValue; + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::Payment; + params[jss::tx_json] = tx_json; - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == - "Invalid field 'tx_blob'."); - } - { - // Non-object `tx_json` - Json::Value params = Json::objectValue; - params[jss::tx_json] = ""; + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Missing field 'tx.Account'."); + } + { + // Empty `tx_blob` + Json::Value params = Json::objectValue; + params[jss::tx_blob] = ""; - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == - "Invalid field 'tx_json', not object."); - } - { - // Invalid transaction - Json::Value params = Json::objectValue; - Json::Value tx_json = Json::objectValue; - tx_json[jss::TransactionType] = jss::Payment; - tx_json[jss::Account] = env.master.human(); - params[jss::tx_json] = tx_json; - - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_exception] == - "Field 'Destination' is required but missing."); - } - { - // Bad account - Json::Value params; - Json::Value tx_json = Json::objectValue; - tx_json[jss::TransactionType] = jss::AccountSet; - tx_json[jss::Account] = "badAccount"; - params[jss::tx_json] = tx_json; + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Invalid field 'tx_blob'."); + } + { + // Non-string `tx_blob` + Json::Value params; + params[jss::tx_blob] = 1.1; - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT(resp[jss::result][jss::error] == "srcActMalformed"); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == - "Invalid field 'tx.Account'."); - } - { - // Account doesn't exist for Sequence autofill - Json::Value params; - Json::Value tx_json = Json::objectValue; - tx_json[jss::TransactionType] = jss::AccountSet; - tx_json[jss::Account] = alice.human(); - params[jss::tx_json] = tx_json; + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Invalid field 'tx_blob'."); + } + { + // Non-object `tx_json` + Json::Value params = Json::objectValue; + params[jss::tx_json] = ""; - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == - "Source account not found."); - } - { - // Invalid transaction - Json::Value params; - Json::Value tx_json = Json::objectValue; - tx_json[jss::TransactionType] = jss::AccountSet; - tx_json[jss::Account] = env.master.human(); - tx_json["foo"] = "bar"; - params[jss::tx_json] = tx_json; + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Invalid field 'tx_json', not object."); + } + { + // Invalid transaction + Json::Value params = Json::objectValue; + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::Payment; + tx_json[jss::Account] = env.master.human(); + params[jss::tx_json] = tx_json; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_exception] == + "Field 'Destination' is required but missing."); + } + { + // Bad account + Json::Value params; + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::AccountSet; + tx_json[jss::Account] = "badAccount"; + params[jss::tx_json] = tx_json; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT(resp[jss::result][jss::error] == "srcActMalformed"); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Invalid field 'tx.Account'."); + } + { + // Account doesn't exist for Sequence autofill + Json::Value params; + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::AccountSet; + tx_json[jss::Account] = alice.human(); + params[jss::tx_json] = tx_json; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Source account not found."); + } + { + // Invalid Signers field + Json::Value params; + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::AccountSet; + tx_json[jss::Account] = env.master.human(); + tx_json[sfSigners.jsonName] = "1"; + params[jss::tx_json] = tx_json; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Invalid field 'tx.Signers'."); + } + { + // Invalid Signers field + Json::Value params; + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::AccountSet; + tx_json[jss::Account] = env.master.human(); + tx_json[sfSigners.jsonName] = Json::arrayValue; + tx_json[sfSigners.jsonName].append("1"); + params[jss::tx_json] = tx_json; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Invalid field 'tx.Signers[0]'."); + } + { + // Invalid transaction + Json::Value params; + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::AccountSet; + tx_json[jss::Account] = env.master.human(); + tx_json["foo"] = "bar"; + params[jss::tx_json] = tx_json; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Field 'tx_json.foo' is unknown."); + } + { + // non-`"binary"` second param for CLI + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::AccountSet; + tx_json[jss::Account] = alice.human(); + auto resp = env.rpc("simulate", to_string(tx_json), "1"); + BEAST_EXPECT(resp[jss::error_message] == "Invalid parameters."); + } + { + // Signed transaction + Json::Value params; + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::AccountSet; + tx_json[jss::Account] = env.master.human(); + tx_json[jss::TxnSignature] = "1200ABCD"; + params[jss::tx_json] = tx_json; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Transaction should not be signed."); + } + { + // Signed multisig transaction + Json::Value params; + Json::Value tx_json = Json::objectValue; + tx_json[jss::TransactionType] = jss::AccountSet; + tx_json[jss::Account] = env.master.human(); + tx_json[sfSigners.jsonName] = Json::arrayValue; + { + Json::Value signer; + signer[jss::Account] = alice.human(); + signer[jss::SigningPubKey] = alice.human(); + signer[jss::TxnSignature] = "1200ABCD"; + Json::Value signerOuter; + signerOuter[sfSigner.jsonName] = signer; + tx_json[sfSigners.jsonName].append(signerOuter); + } + params[jss::tx_json] = tx_json; - auto resp = env.rpc("json", "simulate", to_string(params)); - BEAST_EXPECT( - resp[jss::result][jss::error_message] == - "Field 'tx_json.foo' is unknown."); + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Transaction should not be signed."); + } } + void + testSuccessfulTransaction() { - // non-`"binary"` second param for CLI - Json::Value tx_json = Json::objectValue; - tx_json[jss::TransactionType] = jss::AccountSet; - tx_json[jss::Account] = alice.human(); - auto resp = env.rpc("simulate", to_string(tx_json), "1"); - BEAST_EXPECT(resp[jss::error_message] == "Invalid parameters."); - } -} -void -testSuccessfulTransaction() -{ - testcase("Successful transaction"); + testcase("Successful transaction"); - using namespace jtx; - Env env(*this); - static auto const newDomain = "123ABC"; + using namespace jtx; + Env env(*this); + static auto const newDomain = "123ABC"; - { - auto validateOutput = [&](Json::Value resp, Json::Value& tx) { - auto result = resp[jss::result]; - checkBasicReturnValidity(result, tx); + { + auto validateOutput = [&](Json::Value resp, Json::Value& tx) { + auto result = resp[jss::result]; + checkBasicReturnValidity(result, tx); - BEAST_EXPECT(result[jss::engine_result] == "tesSUCCESS"); - BEAST_EXPECT(result[jss::engine_result_code] == 0); - BEAST_EXPECT( - result[jss::engine_result_message] == - "The simulated transaction would have been applied."); + BEAST_EXPECT(result[jss::engine_result] == "tesSUCCESS"); + BEAST_EXPECT(result[jss::engine_result_code] == 0); + BEAST_EXPECT( + result[jss::engine_result_message] == + "The simulated transaction would have been applied."); - if (BEAST_EXPECT( - result.isMember(jss::meta) || - result.isMember(jss::meta_blob))) - { - Json::Value metadata; - if (result.isMember(jss::meta_blob)) - { - auto unHexed = strUnHex(result[jss::meta_blob].asString()); - SerialIter sitTrans(makeSlice(*unHexed)); - metadata = STObject(std::ref(sitTrans), sfGeneric) - .getJson(JsonOptions::none); - } - else - { - metadata = result[jss::meta]; - } - if (BEAST_EXPECT(metadata.isMember(sfAffectedNodes.jsonName))) + if (BEAST_EXPECT( + result.isMember(jss::meta) || + result.isMember(jss::meta_blob))) { - BEAST_EXPECT( - metadata[sfAffectedNodes.jsonName].size() == 1); - auto node = metadata[sfAffectedNodes.jsonName][0u]; - if (BEAST_EXPECT(node.isMember(sfModifiedNode.jsonName))) + Json::Value metadata; + if (result.isMember(jss::meta_blob)) + { + auto unHexed = + strUnHex(result[jss::meta_blob].asString()); + SerialIter sitTrans(makeSlice(*unHexed)); + metadata = STObject(std::ref(sitTrans), sfGeneric) + .getJson(JsonOptions::none); + } + else + { + metadata = result[jss::meta]; + } + if (BEAST_EXPECT( + metadata.isMember(sfAffectedNodes.jsonName))) { - auto modifiedNode = node[sfModifiedNode.jsonName]; - BEAST_EXPECT( - modifiedNode[sfLedgerEntryType.jsonName] == - "AccountRoot"); - auto finalFields = modifiedNode[sfFinalFields.jsonName]; BEAST_EXPECT( - finalFields[sfDomain.jsonName] == newDomain); + metadata[sfAffectedNodes.jsonName].size() == 1); + auto node = metadata[sfAffectedNodes.jsonName][0u]; + if (BEAST_EXPECT( + node.isMember(sfModifiedNode.jsonName))) + { + auto modifiedNode = node[sfModifiedNode.jsonName]; + BEAST_EXPECT( + modifiedNode[sfLedgerEntryType.jsonName] == + "AccountRoot"); + auto finalFields = + modifiedNode[sfFinalFields.jsonName]; + BEAST_EXPECT( + finalFields[sfDomain.jsonName] == newDomain); + } } + BEAST_EXPECT(metadata[sfTransactionIndex.jsonName] == 0); + BEAST_EXPECT( + metadata[sfTransactionResult.jsonName] == "tesSUCCESS"); } - BEAST_EXPECT(metadata[sfTransactionIndex.jsonName] == 0); - BEAST_EXPECT( - metadata[sfTransactionResult.jsonName] == "tesSUCCESS"); - } - }; + }; - Json::Value tx; + Json::Value tx; - tx[jss::Account] = Account::master.human(); - tx[jss::TransactionType] = jss::AccountSet; - tx[sfDomain.jsonName] = newDomain; + tx[jss::Account] = env.master.human(); + tx[jss::TransactionType] = jss::AccountSet; + tx[sfDomain.jsonName] = newDomain; - // test with autofill - testTx(env, tx, validateOutput); + // test with autofill + testTx(env, tx, validateOutput); - tx[sfSigningPubKey.jsonName] = ""; - tx[sfTxnSignature.jsonName] = ""; - tx[sfSequence.jsonName] = 1; - tx[sfFee.jsonName] = "12"; + tx[sfSigningPubKey.jsonName] = ""; + tx[sfTxnSignature.jsonName] = ""; + tx[sfSequence.jsonName] = 1; + tx[sfFee.jsonName] = "12"; - // test without autofill - testTx(env, tx, validateOutput); + // test without autofill + testTx(env, tx, validateOutput); - // TODO: check that the ledger wasn't affected + // TODO: check that the ledger wasn't affected + } } -} -void -testTransactionNonTecFailure() -{ - testcase("Transaction non-tec failure"); + void + testTransactionNonTecFailure() + { + testcase("Transaction non-tec failure"); - using namespace jtx; - Env env(*this); - Account const alice("alice"); + using namespace jtx; + Env env(*this); + Account const alice("alice"); - { - std::function testSimulation = - [&](Json::Value resp, Json::Value& tx) { - auto result = resp[jss::result]; - checkBasicReturnValidity(result, tx); + { + std::function testSimulation = + [&](Json::Value resp, Json::Value& tx) { + auto result = resp[jss::result]; + checkBasicReturnValidity(result, tx); - BEAST_EXPECT(result[jss::engine_result] == "temBAD_AMOUNT"); - BEAST_EXPECT(result[jss::engine_result_code] == -298); - BEAST_EXPECT( - result[jss::engine_result_message] == - "Can only send positive amounts."); + BEAST_EXPECT(result[jss::engine_result] == "temBAD_AMOUNT"); + BEAST_EXPECT(result[jss::engine_result_code] == -298); + BEAST_EXPECT( + result[jss::engine_result_message] == + "Can only send positive amounts."); - BEAST_EXPECT( - !result.isMember(jss::meta) && - !result.isMember(jss::meta_blob)); - }; + BEAST_EXPECT( + !result.isMember(jss::meta) && + !result.isMember(jss::meta_blob)); + }; - Json::Value tx; + Json::Value tx; - tx[jss::Account] = Account::master.human(); - tx[jss::TransactionType] = jss::Payment; - tx[sfDestination.jsonName] = alice.human(); - tx[sfAmount.jsonName] = "0"; // invalid amount + tx[jss::Account] = env.master.human(); + tx[jss::TransactionType] = jss::Payment; + tx[sfDestination.jsonName] = alice.human(); + tx[sfAmount.jsonName] = "0"; // invalid amount - // test with autofill - testTx(env, tx, testSimulation); + // test with autofill + testTx(env, tx, testSimulation); - tx[sfSigningPubKey.jsonName] = ""; - tx[sfTxnSignature.jsonName] = ""; - tx[sfSequence.jsonName] = 1; - tx[sfFee.jsonName] = "12"; + tx[sfSigningPubKey.jsonName] = ""; + tx[sfTxnSignature.jsonName] = ""; + tx[sfSequence.jsonName] = 1; + tx[sfFee.jsonName] = "12"; - // test without autofill - testTx(env, tx, testSimulation); + // test without autofill + testTx(env, tx, testSimulation); - // TODO: check that the ledger wasn't affected + // TODO: check that the ledger wasn't affected + } } -} -void -testTransactionTecFailure() -{ - testcase("Transaction tec failure"); + void + testTransactionTecFailure() + { + testcase("Transaction tec failure"); - using namespace jtx; - Env env(*this); - Account const alice("alice"); + using namespace jtx; + Env env(*this); + Account const alice("alice"); + { + std::function testSimulation = + [&](Json::Value resp, Json::Value& tx) { + auto result = resp[jss::result]; + checkBasicReturnValidity(result, tx); + + BEAST_EXPECT( + result[jss::engine_result] == "tecNO_DST_INSUF_XRP"); + BEAST_EXPECT(result[jss::engine_result_code] == 125); + BEAST_EXPECT( + result[jss::engine_result_message] == + "Destination does not exist. Too little XRP sent to " + "create " + "it."); + + if (BEAST_EXPECT( + result.isMember(jss::meta) || + result.isMember(jss::meta_blob))) + { + Json::Value metadata; + if (result.isMember(jss::meta_blob)) + { + auto unHexed = + strUnHex(result[jss::meta_blob].asString()); + SerialIter sitTrans(makeSlice(*unHexed)); + metadata = STObject(std::ref(sitTrans), sfGeneric) + .getJson(JsonOptions::none); + } + else + { + metadata = result[jss::meta]; + } + if (BEAST_EXPECT( + metadata.isMember(sfAffectedNodes.jsonName))) + { + BEAST_EXPECT( + metadata[sfAffectedNodes.jsonName].size() == 1); + auto node = metadata[sfAffectedNodes.jsonName][0u]; + if (BEAST_EXPECT( + node.isMember(sfModifiedNode.jsonName))) + { + auto modifiedNode = + node[sfModifiedNode.jsonName]; + BEAST_EXPECT( + modifiedNode[sfLedgerEntryType.jsonName] == + "AccountRoot"); + auto finalFields = + modifiedNode[sfFinalFields.jsonName]; + BEAST_EXPECT( + finalFields[sfBalance.jsonName] == + "99999999999999990"); + } + } + BEAST_EXPECT( + metadata[sfTransactionIndex.jsonName] == 0); + BEAST_EXPECT( + metadata[sfTransactionResult.jsonName] == + "tecNO_DST_INSUF_XRP"); + } + }; + + Json::Value tx; + + tx[jss::Account] = env.master.human(); + tx[jss::TransactionType] = jss::Payment; + tx[sfDestination.jsonName] = alice.human(); + tx[sfAmount.jsonName] = "1"; // not enough to create an account + + // test with autofill + testTx(env, tx, testSimulation); + + tx[sfSigningPubKey.jsonName] = ""; + tx[sfTxnSignature.jsonName] = ""; + tx[sfSequence.jsonName] = 1; + tx[sfFee.jsonName] = "10"; + + // test without autofill + testTx(env, tx, testSimulation); + + // TODO: check that the ledger wasn't affected + } + } + + void + testSuccessfulTransactionMultisigned() { - std::function testSimulation = - [&](Json::Value resp, Json::Value& tx) { + testcase("Successful multi-signed transaction"); + + using namespace jtx; + Env env(*this); + static auto const newDomain = "123ABC"; + Account const alice("alice"); + Account const becky("becky"); + Account const carol("carol"); + env.fund(XRP(10000), alice); + env.close(); + + // set up valid multisign + env(signers(alice, 1, {{becky, 1}, {carol, 1}})); + + { + auto validateOutput = [&](Json::Value resp, Json::Value& tx) { auto result = resp[jss::result]; - checkBasicReturnValidity(result, tx); + checkBasicReturnValidity(result, tx, "20", 5); - BEAST_EXPECT( - result[jss::engine_result] == "tecNO_DST_INSUF_XRP"); - BEAST_EXPECT(result[jss::engine_result_code] == 125); + BEAST_EXPECT(result[jss::engine_result] == "tesSUCCESS"); + BEAST_EXPECT(result[jss::engine_result_code] == 0); BEAST_EXPECT( result[jss::engine_result_message] == - "Destination does not exist. Too little XRP sent to create " - "it."); + "The simulated transaction would have been applied."); if (BEAST_EXPECT( result.isMember(jss::meta) || @@ -431,49 +609,58 @@ testTransactionTecFailure() auto finalFields = modifiedNode[sfFinalFields.jsonName]; BEAST_EXPECT( - finalFields[sfBalance.jsonName] == - "99999999999999990"); + finalFields[sfDomain.jsonName] == newDomain); } } - BEAST_EXPECT(metadata[sfTransactionIndex.jsonName] == 0); + BEAST_EXPECT(metadata[sfTransactionIndex.jsonName] == 1); BEAST_EXPECT( - metadata[sfTransactionResult.jsonName] == - "tecNO_DST_INSUF_XRP"); + metadata[sfTransactionResult.jsonName] == "tesSUCCESS"); } }; - Json::Value tx; + Json::Value tx; - tx[jss::Account] = Account::master.human(); - tx[jss::TransactionType] = jss::Payment; - tx[sfDestination.jsonName] = alice.human(); - tx[sfAmount.jsonName] = "1"; // not enough to create an account + tx[jss::Account] = alice.human(); + tx[jss::TransactionType] = jss::AccountSet; + tx[sfDomain.jsonName] = newDomain; + tx[sfSigners.jsonName] = Json::arrayValue; + { + Json::Value signer; + signer[jss::Account] = becky.human(); + signer[jss::SigningPubKey] = strHex(becky.pk().slice()); + Json::Value signerOuter; + signerOuter[sfSigner.jsonName] = signer; + tx[sfSigners.jsonName].append(signerOuter); + } - // test with autofill - testTx(env, tx, testSimulation); + // test with autofill + testTx(env, tx, validateOutput, false); - tx[sfSigningPubKey.jsonName] = ""; - tx[sfTxnSignature.jsonName] = ""; - tx[sfSequence.jsonName] = 1; - tx[sfFee.jsonName] = "10"; + tx[sfSigningPubKey.jsonName] = ""; + tx[sfTxnSignature.jsonName] = ""; + tx[sfSequence.jsonName] = 5; + tx[sfFee.jsonName] = "20"; + tx[sfSigners.jsonName][0u][sfSigner.jsonName][jss::TxnSignature] = + ""; - // test without autofill - testTx(env, tx, testSimulation); + // test without autofill + testTx(env, tx, validateOutput); - // TODO: check that the ledger wasn't affected + // TODO: check that the ledger wasn't affected + } } -} public: -void -run() override -{ - testParamErrors(); - testSuccessfulTransaction(); - testTransactionNonTecFailure(); - testTransactionTecFailure(); -} -}; // namespace ripple + void + run() override + { + testParamErrors(); + testSuccessfulTransaction(); + testTransactionNonTecFailure(); + testTransactionTecFailure(); + testSuccessfulTransactionMultisigned(); + } +}; BEAST_DEFINE_TESTSUITE(Simulate, rpc, ripple); diff --git a/src/xrpld/rpc/detail/TransactionSign.cpp b/src/xrpld/rpc/detail/TransactionSign.cpp index bbd1ddc64b7..1ce6e680d29 100644 --- a/src/xrpld/rpc/detail/TransactionSign.cpp +++ b/src/xrpld/rpc/detail/TransactionSign.cpp @@ -730,14 +730,13 @@ getBaseFee(Application const& app, Config const& config, Json::Value tx) if (tx.isMember(sfSigners.jsonName)) { if (!tx[sfSigners.jsonName].isArray()) - Throw(RPC::invalid_field_message("tx.Signers")); + return config.FEES.reference_fee; // check multisigned signers for (auto& signer : tx[sfSigners.jsonName]) { if (!signer.isMember(sfSigner.jsonName) || !signer[sfSigner.jsonName].isObject()) - Throw( - RPC::invalid_field_message("tx.Signers")); + return config.FEES.reference_fee; if (!signer[sfSigner.jsonName].isMember(sfTxnSignature.jsonName)) { // autofill TxnSignature diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index 8b042f309d6..bca28d4af05 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -36,6 +36,22 @@ namespace ripple { std::optional autofillTx(Json::Value& tx_json, RPC::JsonContext& context) { + if (!tx_json.isMember(jss::Fee)) + { + // autofill Fee + // Must happen after all the other autofills happen + // Error handling/messaging works better that way + auto feeOrError = RPC::getCurrentFee( + context.role, + context.app.config(), + context.app.getFeeTrack(), + context.app.getTxQ(), + context.app, + tx_json); + if (feeOrError.isMember(jss::error)) + return feeOrError; + tx_json[jss::Fee] = feeOrError; + } if (!tx_json.isMember(sfSigningPubKey.jsonName)) { // autofill SigningPubKey @@ -46,11 +62,13 @@ autofillTx(Json::Value& tx_json, RPC::JsonContext& context) if (!tx_json[sfSigners.jsonName].isArray()) return RPC::invalid_field_error("tx.Signers"); // check multisigned signers - for (auto& signer : tx_json[sfSigners.jsonName]) + for (int index = 0; index < tx_json[sfSigners.jsonName].size(); index++) { - if (!signer.isMember(sfSigner.jsonName) || + auto& signer = tx_json[sfSigners.jsonName][index]; + if (!signer.isObject() || !signer.isMember(sfSigner.jsonName) || !signer[sfSigner.jsonName].isObject()) - return RPC::invalid_field_error("tx.Signers"); + return RPC::invalid_field_error( + "tx.Signers[" + std::to_string(index) + "]"); if (!signer[sfSigner.jsonName].isMember(sfTxnSignature.jsonName)) { // autofill TxnSignature @@ -105,22 +123,6 @@ autofillTx(Json::Value& tx_json, RPC::JsonContext& context) ? 0 : context.app.getTxQ().nextQueuableSeq(sle).value(); } - if (!tx_json.isMember(jss::Fee)) - { - // autofill Fee - // Must happen after all the other autofills happen - // Error handling/messaging works better that way - auto feeOrError = RPC::getCurrentFee( - context.role, - context.app.config(), - context.app.getFeeTrack(), - context.app.getTxQ(), - context.app, - tx_json); - if (feeOrError.isMember(jss::error)) - return feeOrError; - tx_json[jss::Fee] = feeOrError; - } return std::nullopt; } From 4aa5a3f76875d325e134c7e4652763340347c1d9 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 23 Jul 2024 12:32:12 -0400 Subject: [PATCH 31/42] add comment --- src/test/rpc/Simulate_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp index 1e857c90145..90b6e0b32cd 100644 --- a/src/test/rpc/Simulate_test.cpp +++ b/src/test/rpc/Simulate_test.cpp @@ -639,7 +639,7 @@ class Simulate_test : public beast::unit_test::suite tx[sfSigningPubKey.jsonName] = ""; tx[sfTxnSignature.jsonName] = ""; tx[sfSequence.jsonName] = 5; - tx[sfFee.jsonName] = "20"; + tx[sfFee.jsonName] = "20"; // also tests a non-base fee tx[sfSigners.jsonName][0u][sfSigner.jsonName][jss::TxnSignature] = ""; From a0c472f47f9ff412d379045c702d9bb9206976dd Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 23 Jul 2024 13:19:21 -0400 Subject: [PATCH 32/42] add test for sign-and-submit higher base fee tx --- src/test/rpc/JSONRPC_test.cpp | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/test/rpc/JSONRPC_test.cpp b/src/test/rpc/JSONRPC_test.cpp index b2b9e7a55b8..cee2bd5774b 100644 --- a/src/test/rpc/JSONRPC_test.cpp +++ b/src/test/rpc/JSONRPC_test.cpp @@ -2160,6 +2160,31 @@ class JSONRPC_test : public beast::unit_test::suite BEAST_EXPECT(RPC::contains_error(result)); BEAST_EXPECT(!req[jss::tx_json].isMember(jss::Fee)); } + + { + // transaction with a higher base fee + Json::Value req; + test::jtx::Account const alice("alice"); + Json::Reader().parse( + "{ \"tx_json\" : { \"TransactionType\": \"AccountDelete\", " + "\"Account\": \"" + + env.master.human() + "\", \"Destination\": \"" + + alice.human() + "\" } } ", + req); + Json::Value result = checkFee( + req, + Role::ADMIN, + true, + env.app().config(), + feeTrack, + env.app().getTxQ(), + env.app()); + + BEAST_EXPECT(!RPC::contains_error(result)); + BEAST_EXPECT( + req[jss::tx_json].isMember(jss::Fee) && + req[jss::tx_json][jss::Fee] == 50000000); + } } void From b3ec9355ea72914accf7ed08d3de548116ba52f5 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 23 Jul 2024 13:28:45 -0400 Subject: [PATCH 33/42] add NetworkID support --- src/test/rpc/Simulate_test.cpp | 29 +++++++++++++++++++++++++++-- src/xrpld/rpc/handlers/Simulate.cpp | 5 +++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp index 90b6e0b32cd..6c6a6ae8ffb 100644 --- a/src/test/rpc/Simulate_test.cpp +++ b/src/test/rpc/Simulate_test.cpp @@ -37,6 +37,16 @@ namespace test { class Simulate_test : public beast::unit_test::suite { + std::unique_ptr + makeNetworkConfig(uint32_t networkID) + { + using namespace jtx; + return envconfig([&](std::unique_ptr cfg) { + cfg->NETWORK_ID = networkID; + return cfg; + }); + } + void checkBasicReturnValidity( Json::Value& result, @@ -411,10 +421,11 @@ class Simulate_test : public beast::unit_test::suite void testTransactionNonTecFailure() { - testcase("Transaction non-tec failure"); + testcase("Transaction non-tec failure + NetworkID autofill"); using namespace jtx; - Env env(*this); + // also check NetworkID + test::jtx::Env env{*this, makeNetworkConfig(1025)}; Account const alice("alice"); { @@ -422,6 +433,20 @@ class Simulate_test : public beast::unit_test::suite [&](Json::Value resp, Json::Value& tx) { auto result = resp[jss::result]; checkBasicReturnValidity(result, tx); + Json::Value tx_json; + if (result.isMember(jss::tx_json)) + { + tx_json = result[jss::tx_json]; + } + else + { + auto unHexed = + strUnHex(result[jss::tx_blob].asString()); + SerialIter sitTrans(makeSlice(*unHexed)); + tx_json = STObject(std::ref(sitTrans), sfGeneric) + .getJson(JsonOptions::none); + } + BEAST_EXPECT(tx_json.get(jss::NetworkID, -1) == 1025); BEAST_EXPECT(result[jss::engine_result] == "temBAD_AMOUNT"); BEAST_EXPECT(result[jss::engine_result_code] == -298); diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index bca28d4af05..c0cdf529872 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -123,6 +123,11 @@ autofillTx(Json::Value& tx_json, RPC::JsonContext& context) ? 0 : context.app.getTxQ().nextQueuableSeq(sle).value(); } + if (!tx_json.isMember(jss::NetworkID) && + context.app.config().NETWORK_ID > 1024) + { + tx_json[jss::NetworkID] = context.app.config().NETWORK_ID; + } return std::nullopt; } From d712dc9974a4574533f71793c3657ddb1ff9282e Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 23 Jul 2024 13:47:48 -0400 Subject: [PATCH 34/42] add Signers and Signer to jss --- include/xrpl/protocol/jss.h | 2 ++ src/libxrpl/protocol/InnerObjectFormats.cpp | 2 +- src/xrpld/rpc/detail/TransactionSign.cpp | 16 ++++++++-------- src/xrpld/rpc/handlers/Simulate.cpp | 18 +++++++++--------- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/include/xrpl/protocol/jss.h b/include/xrpl/protocol/jss.h index a46e15f39ef..9e3be62efe5 100644 --- a/include/xrpl/protocol/jss.h +++ b/include/xrpl/protocol/jss.h @@ -137,8 +137,10 @@ JSS(SendMax); // in: TransactionSign JSS(Sequence); // in/out: TransactionSign; field. JSS(SetFlag); // field. JSS(SetRegularKey); // transaction type. +JSS(Signer); // field. JSS(SignerList); // ledger type. JSS(SignerListSet); // transaction type. +JSS(Signers); // field. JSS(SigningPubKey); // field. JSS(TakerGets); // field. JSS(TakerPays); // field. diff --git a/src/libxrpl/protocol/InnerObjectFormats.cpp b/src/libxrpl/protocol/InnerObjectFormats.cpp index 6d7b855d199..41a4305657c 100644 --- a/src/libxrpl/protocol/InnerObjectFormats.cpp +++ b/src/libxrpl/protocol/InnerObjectFormats.cpp @@ -36,7 +36,7 @@ InnerObjectFormats::InnerObjectFormats() {sfWalletLocator, soeOPTIONAL}, }); - add(sfSigner.jsonName.c_str(), + add(sfSigner.jsonName, sfSigner.getCode(), { {sfAccount, soeREQUIRED}, diff --git a/src/xrpld/rpc/detail/TransactionSign.cpp b/src/xrpld/rpc/detail/TransactionSign.cpp index 1ce6e680d29..c55469d77f3 100644 --- a/src/xrpld/rpc/detail/TransactionSign.cpp +++ b/src/xrpld/rpc/detail/TransactionSign.cpp @@ -504,7 +504,7 @@ transactionPreProcessImpl( } else if (signingArgs.isSingleSigning()) { - if (tx_json.isMember(sfSigners.jsonName)) + if (tx_json.isMember(jss::Signers)) return rpcError(rpcALREADY_MULTISIG); } @@ -727,20 +727,20 @@ getBaseFee(Application const& app, Config const& config, Json::Value tx) { tx[jss::TxnSignature] = ""; } - if (tx.isMember(sfSigners.jsonName)) + if (tx.isMember(jss::Signers)) { - if (!tx[sfSigners.jsonName].isArray()) + if (!tx[jss::Signers].isArray()) return config.FEES.reference_fee; // check multisigned signers - for (auto& signer : tx[sfSigners.jsonName]) + for (auto& signer : tx[jss::Signers]) { - if (!signer.isMember(sfSigner.jsonName) || - !signer[sfSigner.jsonName].isObject()) + if (!signer.isMember(jss::Signer) || + !signer[jss::Signer].isObject()) return config.FEES.reference_fee; - if (!signer[sfSigner.jsonName].isMember(sfTxnSignature.jsonName)) + if (!signer[jss::Signer].isMember(sfTxnSignature.jsonName)) { // autofill TxnSignature - signer[sfSigner.jsonName][sfTxnSignature.jsonName] = ""; + signer[jss::Signer][sfTxnSignature.jsonName] = ""; } } } diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index c0cdf529872..75e3564be8c 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -57,24 +57,24 @@ autofillTx(Json::Value& tx_json, RPC::JsonContext& context) // autofill SigningPubKey tx_json[sfSigningPubKey.jsonName] = ""; } - if (tx_json.isMember(sfSigners.jsonName)) + if (tx_json.isMember(jss::Signers)) { - if (!tx_json[sfSigners.jsonName].isArray()) + if (!tx_json[jss::Signers].isArray()) return RPC::invalid_field_error("tx.Signers"); // check multisigned signers - for (int index = 0; index < tx_json[sfSigners.jsonName].size(); index++) + for (int index = 0; index < tx_json[jss::Signers].size(); index++) { - auto& signer = tx_json[sfSigners.jsonName][index]; - if (!signer.isObject() || !signer.isMember(sfSigner.jsonName) || - !signer[sfSigner.jsonName].isObject()) + auto& signer = tx_json[jss::Signers][index]; + if (!signer.isObject() || !signer.isMember(jss::Signer) || + !signer[jss::Signer].isObject()) return RPC::invalid_field_error( "tx.Signers[" + std::to_string(index) + "]"); - if (!signer[sfSigner.jsonName].isMember(sfTxnSignature.jsonName)) + if (!signer[jss::Signer].isMember(sfTxnSignature.jsonName)) { // autofill TxnSignature - signer[sfSigner.jsonName][sfTxnSignature.jsonName] = ""; + signer[jss::Signer][sfTxnSignature.jsonName] = ""; } - else if (signer[sfSigner.jsonName][sfTxnSignature.jsonName] != "") + else if (signer[jss::Signer][sfTxnSignature.jsonName] != "") { // Transaction must not be signed return rpcError(rpcTX_SIGNED); From 834965bad6eab155190d6e68e3e42ae2b6bfba97 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 23 Jul 2024 14:35:34 -0400 Subject: [PATCH 35/42] Revert "add NetworkID support" This reverts commit b3ec9355ea72914accf7ed08d3de548116ba52f5. --- src/test/rpc/Simulate_test.cpp | 29 ++--------------------------- src/xrpld/rpc/handlers/Simulate.cpp | 5 ----- 2 files changed, 2 insertions(+), 32 deletions(-) diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp index 6c6a6ae8ffb..90b6e0b32cd 100644 --- a/src/test/rpc/Simulate_test.cpp +++ b/src/test/rpc/Simulate_test.cpp @@ -37,16 +37,6 @@ namespace test { class Simulate_test : public beast::unit_test::suite { - std::unique_ptr - makeNetworkConfig(uint32_t networkID) - { - using namespace jtx; - return envconfig([&](std::unique_ptr cfg) { - cfg->NETWORK_ID = networkID; - return cfg; - }); - } - void checkBasicReturnValidity( Json::Value& result, @@ -421,11 +411,10 @@ class Simulate_test : public beast::unit_test::suite void testTransactionNonTecFailure() { - testcase("Transaction non-tec failure + NetworkID autofill"); + testcase("Transaction non-tec failure"); using namespace jtx; - // also check NetworkID - test::jtx::Env env{*this, makeNetworkConfig(1025)}; + Env env(*this); Account const alice("alice"); { @@ -433,20 +422,6 @@ class Simulate_test : public beast::unit_test::suite [&](Json::Value resp, Json::Value& tx) { auto result = resp[jss::result]; checkBasicReturnValidity(result, tx); - Json::Value tx_json; - if (result.isMember(jss::tx_json)) - { - tx_json = result[jss::tx_json]; - } - else - { - auto unHexed = - strUnHex(result[jss::tx_blob].asString()); - SerialIter sitTrans(makeSlice(*unHexed)); - tx_json = STObject(std::ref(sitTrans), sfGeneric) - .getJson(JsonOptions::none); - } - BEAST_EXPECT(tx_json.get(jss::NetworkID, -1) == 1025); BEAST_EXPECT(result[jss::engine_result] == "temBAD_AMOUNT"); BEAST_EXPECT(result[jss::engine_result_code] == -298); diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index 75e3564be8c..f2acc428779 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -123,11 +123,6 @@ autofillTx(Json::Value& tx_json, RPC::JsonContext& context) ? 0 : context.app.getTxQ().nextQueuableSeq(sle).value(); } - if (!tx_json.isMember(jss::NetworkID) && - context.app.config().NETWORK_ID > 1024) - { - tx_json[jss::NetworkID] = context.app.config().NETWORK_ID; - } return std::nullopt; } From 22fdaf7e13c9d3a2fdd63271fec050e6a32d2d22 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 24 Jul 2024 13:05:39 -0400 Subject: [PATCH 36/42] handle blob errors better --- src/test/rpc/Simulate_test.cpp | 10 ++++++++++ src/xrpld/rpc/handlers/Simulate.cpp | 14 ++++++++++---- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp index 90b6e0b32cd..58df85dd94f 100644 --- a/src/test/rpc/Simulate_test.cpp +++ b/src/test/rpc/Simulate_test.cpp @@ -144,6 +144,16 @@ class Simulate_test : public beast::unit_test::suite resp[jss::result][jss::error_message] == "Invalid field 'binary'."); } + { + // Invalid `tx_blob` + Json::Value params = Json::objectValue; + params[jss::tx_blob] = "12"; + + auto resp = env.rpc("json", "simulate", to_string(params)); + BEAST_EXPECT( + resp[jss::result][jss::error_message] == + "Invalid field 'tx_blob'."); + } { // Empty `tx_json` Json::Value params = Json::objectValue; diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index f2acc428779..19242ea9038 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -167,10 +167,16 @@ doSimulate(RPC::JsonContext& context) if (!unHexed || !unHexed->size()) return RPC::invalid_field_error(jss::tx_blob); - - SerialIter sitTrans(makeSlice(*unHexed)); - tx_json = - STObject(std::ref(sitTrans), sfGeneric).getJson(JsonOptions::none); + try + { + SerialIter sitTrans(makeSlice(*unHexed)); + tx_json = STObject(std::ref(sitTrans), sfGeneric) + .getJson(JsonOptions::none); + } + catch (std::runtime_error& e) + { + return RPC::invalid_field_error(jss::tx_blob); + } } else if (context.params.isMember(jss::tx_json)) { From 4c1f93f6e0b4a050e3572c865d2579ebecca095b Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 30 Jul 2024 17:00:57 -0400 Subject: [PATCH 37/42] remove unneeded check --- src/xrpld/app/tx/detail/Transactor.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index 7fd82eecd37..2be334ff6ee 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -488,11 +488,6 @@ Transactor::apply() NotTEC Transactor::checkSign(PreclaimContext const& ctx) { - if (ctx.flags & tapDRY_RUN && ctx.tx.getSigningPubKey().empty() && - ctx.tx.getFieldArray(sfSigners).empty()) - { - return tesSUCCESS; - } // If the pk is empty, then we must be multi-signing. if (ctx.tx.getSigningPubKey().empty()) return checkMultiSign(ctx); From 270fd02388e906844baa9afbf0a93853e7e8fabd Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Tue, 30 Jul 2024 17:49:53 -0400 Subject: [PATCH 38/42] fix tests --- src/xrpld/app/tx/detail/Transactor.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index 2be334ff6ee..b97366139ca 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -488,6 +488,14 @@ Transactor::apply() NotTEC Transactor::checkSign(PreclaimContext const& ctx) { + if (ctx.flags & tapDRY_RUN) + { + // This code must be different for `simulate` + // Since the public key may be empty even for single signing + if (ctx.tx.isFieldPresent(sfSigners)) + return checkMultiSign(ctx); + return checkSingleSign(ctx); + } // If the pk is empty, then we must be multi-signing. if (ctx.tx.getSigningPubKey().empty()) return checkMultiSign(ctx); @@ -500,7 +508,7 @@ Transactor::checkSingleSign(PreclaimContext const& ctx) { // Check that the value in the signing key slot is a public key. auto const pkSigner = ctx.tx.getSigningPubKey(); - if (!publicKeyType(makeSlice(pkSigner))) + if (!(ctx.flags & tapDRY_RUN) && !publicKeyType(makeSlice(pkSigner))) { JLOG(ctx.j.trace()) << "checkSingleSign: signing public key type is unknown"; @@ -508,8 +516,11 @@ Transactor::checkSingleSign(PreclaimContext const& ctx) } // Look up the account. - auto const idSigner = calcAccountID(PublicKey(makeSlice(pkSigner))); auto const idAccount = ctx.tx.getAccountID(sfAccount); + // This ternary is only needed to handle `simulate` + auto const idSigner = pkSigner.size() > 0 + ? calcAccountID(PublicKey(makeSlice(pkSigner))) + : idAccount; auto const sleAccount = ctx.view.read(keylet::account(idAccount)); if (!sleAccount) return terNO_ACCOUNT; From a3510c4d5207abeff94a19a25c43f98b2460ea76 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 31 Jul 2024 12:55:15 -0400 Subject: [PATCH 39/42] add signing failure test --- src/test/rpc/Simulate_test.cpp | 59 ++++++++++++++++++++++++++++++++-- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp index 58df85dd94f..d4b31547f3a 100644 --- a/src/test/rpc/Simulate_test.cpp +++ b/src/test/rpc/Simulate_test.cpp @@ -41,8 +41,8 @@ class Simulate_test : public beast::unit_test::suite checkBasicReturnValidity( Json::Value& result, Json::Value& tx, - std::string fee = "10", - int sequence = 1) + int sequence = 1, + std::string fee = "10") { BEAST_EXPECT(result[jss::applied] == false); BEAST_EXPECT(result.isMember(jss::engine_result)); @@ -578,7 +578,7 @@ class Simulate_test : public beast::unit_test::suite { auto validateOutput = [&](Json::Value resp, Json::Value& tx) { auto result = resp[jss::result]; - checkBasicReturnValidity(result, tx, "20", 5); + checkBasicReturnValidity(result, tx, 5, "20"); BEAST_EXPECT(result[jss::engine_result] == "tesSUCCESS"); BEAST_EXPECT(result[jss::engine_result_code] == 0); @@ -660,6 +660,58 @@ class Simulate_test : public beast::unit_test::suite } } + void + testTransactionSigningFailure() + { + testcase("Transaction with a key-related failure"); + + using namespace jtx; + Env env(*this); + static auto const newDomain = "123ABC"; + Account const alice{"alice"}; + env(regkey(env.master, alice)); + env(fset(env.master, asfDisableMaster), sig(env.master)); + env.close(); + + { + std::function testSimulation = + [&](Json::Value resp, Json::Value& tx) { + auto result = resp[jss::result]; + checkBasicReturnValidity(result, tx, 3); + + BEAST_EXPECT( + result[jss::engine_result] == "tefMASTER_DISABLED"); + BEAST_EXPECT(result[jss::engine_result_code] == -188); + BEAST_EXPECT( + result[jss::engine_result_message] == + "Master key is disabled."); + + BEAST_EXPECT( + !result.isMember(jss::meta) && + !result.isMember(jss::meta_blob)); + }; + + Json::Value tx; + + tx[jss::Account] = env.master.human(); + tx[jss::TransactionType] = jss::AccountSet; + tx[sfDomain.jsonName] = newDomain; + + // test with autofill + testTx(env, tx, testSimulation, 3); + + tx[sfSigningPubKey.jsonName] = ""; + tx[sfTxnSignature.jsonName] = ""; + tx[sfSequence.jsonName] = 3; + tx[sfFee.jsonName] = "12"; + + // test without autofill + testTx(env, tx, testSimulation, 3); + + // TODO: check that the ledger wasn't affected + } + } + public: void run() override @@ -669,6 +721,7 @@ class Simulate_test : public beast::unit_test::suite testTransactionNonTecFailure(); testTransactionTecFailure(); testSuccessfulTransactionMultisigned(); + testTransactionSigningFailure(); } }; From 6108fd94646c2ddfabddd5d2efcf8affa13b6c43 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Mon, 28 Oct 2024 16:55:59 -0400 Subject: [PATCH 40/42] fix merge issues --- include/xrpl/protocol/jss.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/xrpl/protocol/jss.h b/include/xrpl/protocol/jss.h index 85d2d37b940..0e7bcdffbdd 100644 --- a/include/xrpl/protocol/jss.h +++ b/include/xrpl/protocol/jss.h @@ -102,10 +102,8 @@ JSS(SettleDelay); // in: TransactionSign JSS(SendMax); // in: TransactionSign JSS(Sequence); // in/out: TransactionSign; field. JSS(SetFlag); // field. -JSS(SetRegularKey); // transaction type. JSS(Signer); // field. JSS(SignerList); // ledger type. -JSS(SignerListSet); // transaction type. JSS(Signers); // field. JSS(SigningPubKey); // field. JSS(TakerGets); // field. From ea59f60ccb178d0a98278b834bca24924d6f75f3 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Wed, 6 Nov 2024 18:37:19 -0500 Subject: [PATCH 41/42] fix spacing, compile error --- src/libxrpl/protocol/ErrorCodes.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libxrpl/protocol/ErrorCodes.cpp b/src/libxrpl/protocol/ErrorCodes.cpp index 10ca977b128..388aac1484a 100644 --- a/src/libxrpl/protocol/ErrorCodes.cpp +++ b/src/libxrpl/protocol/ErrorCodes.cpp @@ -109,8 +109,8 @@ constexpr static ErrorInfo unorderedErrorInfos[]{ {rpcTXN_NOT_FOUND, "txnNotFound", "Transaction not found.", 404}, {rpcUNKNOWN_COMMAND, "unknownCmd", "Unknown method.", 405}, {rpcORACLE_MALFORMED, "oracleMalformed", "Oracle request is malformed.", 400}, - {rpcBAD_CREDENTIALS, "badCredentials", "Credentials do not exist, are not accepted, or have expired.", 400}}; - {rpcTX_SIGNED, "transactionSigned", "Transaction should not be signed.", 400}}; + {rpcBAD_CREDENTIALS, "badCredentials", "Credentials do not exist, are not accepted, or have expired.", 400}, + {rpcTX_SIGNED, "transactionSigned", "Transaction should not be signed.", 400}}; // clang-format on // Sort and validate unorderedErrorInfos at compile time. Should be From 92bd7518c1835e605c7fd23db04fd82eb882ba18 Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Thu, 7 Nov 2024 11:33:06 -0500 Subject: [PATCH 42/42] fix tests --- src/test/rpc/Simulate_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp index d4b31547f3a..a4cb0d48daf 100644 --- a/src/test/rpc/Simulate_test.cpp +++ b/src/test/rpc/Simulate_test.cpp @@ -437,7 +437,7 @@ class Simulate_test : public beast::unit_test::suite BEAST_EXPECT(result[jss::engine_result_code] == -298); BEAST_EXPECT( result[jss::engine_result_message] == - "Can only send positive amounts."); + "Malformed: Bad amount."); BEAST_EXPECT( !result.isMember(jss::meta) &&