diff --git a/src/test/rpc/Simulate_test.cpp b/src/test/rpc/Simulate_test.cpp index 5855d7ccf0d..6ba5562da24 100644 --- a/src/test/rpc/Simulate_test.cpp +++ b/src/test/rpc/Simulate_test.cpp @@ -149,7 +149,8 @@ class Simulate_test : public beast::unit_test::suite Json::Value const params = Json::objectValue; auto const resp = env.rpc("json", "simulate", to_string(params)); BEAST_EXPECT( - resp[jss::result][jss::error_message] == "Invalid parameters."); + resp[jss::result][jss::error_message] == + "Neither `tx_blob` nor `tx_json` included."); } { // Providing both `tx_json` and `tx_blob` @@ -159,7 +160,8 @@ class Simulate_test : public beast::unit_test::suite auto const resp = env.rpc("json", "simulate", to_string(params)); BEAST_EXPECT( - resp[jss::result][jss::error_message] == "Invalid parameters."); + resp[jss::result][jss::error_message] == + "Can only include one of `tx_blob` and `tx_json`."); } { // `binary` isn't a boolean @@ -627,7 +629,7 @@ class Simulate_test : public beast::unit_test::suite Json::Value const& tx) { auto result = resp[jss::result]; checkBasicReturnValidity( - result, tx, 5, env.current()->fees().base * 2); + result, tx, env.seq(alice), env.current()->fees().base * 2); BEAST_EXPECT(result[jss::engine_result] == "tesSUCCESS"); BEAST_EXPECT(result[jss::engine_result_code] == 0); @@ -716,7 +718,10 @@ class Simulate_test : public beast::unit_test::suite [&](Json::Value const& resp, Json::Value const& tx) { auto result = resp[jss::result]; checkBasicReturnValidity( - result, tx, 3, env.current()->fees().base); + result, + tx, + env.seq(env.master), + env.current()->fees().base); BEAST_EXPECT( result[jss::engine_result] == "tefMASTER_DISABLED"); @@ -737,7 +742,7 @@ class Simulate_test : public beast::unit_test::suite tx[sfDomain] = newDomain; // test with autofill - testTx(env, tx, testSimulation, 3); + testTx(env, tx, testSimulation); tx[sfSigningPubKey] = ""; tx[sfTxnSignature] = ""; @@ -745,7 +750,7 @@ class Simulate_test : public beast::unit_test::suite tx[sfFee] = env.current()->fees().base.jsonClipped().asString(); // test without autofill - testTx(env, tx, testSimulation, 3); + testTx(env, tx, testSimulation); // TODO: check that the ledger wasn't affected } @@ -774,7 +779,7 @@ class Simulate_test : public beast::unit_test::suite Json::Value const& tx) { auto result = resp[jss::result]; checkBasicReturnValidity( - result, tx, 5, env.current()->fees().base * 2); + result, tx, env.seq(alice), env.current()->fees().base * 2); BEAST_EXPECTS( result[jss::engine_result] == "tefBAD_SIGNATURE", @@ -852,7 +857,7 @@ class Simulate_test : public beast::unit_test::suite Json::Value const& tx) { auto result = resp[jss::result]; checkBasicReturnValidity( - result, tx, 4, env.current()->fees().base); + result, tx, env.seq(subject), env.current()->fees().base); BEAST_EXPECT(result[jss::engine_result] == "tecEXPIRED"); BEAST_EXPECT(result[jss::engine_result_code] == 148); @@ -864,19 +869,8 @@ class Simulate_test : public beast::unit_test::suite 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]; - } + Json::Value const metadata = getJsonMetadata(result); + if (BEAST_EXPECT( metadata.isMember(sfAffectedNodes.jsonName))) { diff --git a/src/xrpld/rpc/detail/TransactionSign.cpp b/src/xrpld/rpc/detail/TransactionSign.cpp index 4c13a910c2a..37a84838810 100644 --- a/src/xrpld/rpc/detail/TransactionSign.cpp +++ b/src/xrpld/rpc/detail/TransactionSign.cpp @@ -540,7 +540,7 @@ transactionPreProcessImpl( return err; } - std::shared_ptr stpTrans; + std::shared_ptr stTx; try { // If we're generating a multi-signature the SigningPubKey must be @@ -549,7 +549,7 @@ transactionPreProcessImpl( sfSigningPubKey, signingArgs.isMultiSigning() ? Slice(nullptr, 0) : pk.slice()); - stpTrans = std::make_shared(std::move(parsed.object.value())); + stTx = std::make_shared(std::move(parsed.object.value())); } catch (STObject::FieldErr& err) { @@ -563,14 +563,13 @@ transactionPreProcessImpl( } std::string reason; - if (!passesLocalChecks(*stpTrans, reason)) + if (!passesLocalChecks(*stTx, reason)) return RPC::make_error(rpcINVALID_PARAMS, reason); // If multisign then return multiSignature, else set TxnSignature field. if (signingArgs.isMultiSigning()) { - Serializer s = - buildMultiSigningData(*stpTrans, signingArgs.getSigner()); + Serializer s = buildMultiSigningData(*stTx, signingArgs.getSigner()); auto multisig = ripple::sign(pk, sk, s.slice()); @@ -578,15 +577,15 @@ transactionPreProcessImpl( } else if (signingArgs.isSingleSigning()) { - stpTrans->sign(pk, sk); + stTx->sign(pk, sk); } - return transactionPreProcessResult{std::move(stpTrans)}; + return transactionPreProcessResult{std::move(stTx)}; } static std::pair transactionConstructImpl( - std::shared_ptr const& stpTrans, + std::shared_ptr const& stTx, Rules const& rules, Application& app) { @@ -596,7 +595,7 @@ transactionConstructImpl( Transaction::pointer tpTrans; { std::string reason; - tpTrans = std::make_shared(stpTrans, reason, app); + tpTrans = std::make_shared(stTx, reason, app); if (tpTrans->getStatus() != NEW) { ret.first = RPC::make_error( @@ -1228,7 +1227,7 @@ transactionSubmitMultiSigned( } // Grind through the JSON in tx_json to produce a STTx. - std::shared_ptr stpTrans; + std::shared_ptr stTx; { STParsedJSONObject parsedTx_json("tx_json", tx_json); if (!parsedTx_json.object) @@ -1241,7 +1240,7 @@ transactionSubmitMultiSigned( } try { - stpTrans = + stTx = std::make_shared(std::move(parsedTx_json.object.value())); } catch (STObject::FieldErr& err) @@ -1256,7 +1255,7 @@ transactionSubmitMultiSigned( "Exception while serializing transaction: " + reason); } std::string reason; - if (!passesLocalChecks(*stpTrans, reason)) + if (!passesLocalChecks(*stTx, reason)) return RPC::make_error(rpcINVALID_PARAMS, reason); } @@ -1266,7 +1265,7 @@ transactionSubmitMultiSigned( // Verify the values of select fields. // // The SigningPubKey must be present but empty. - if (!stpTrans->getFieldVL(sfSigningPubKey).empty()) + if (!stTx->getFieldVL(sfSigningPubKey).empty()) { std::ostringstream err; err << "Invalid " << sfSigningPubKey.fieldName @@ -1275,11 +1274,11 @@ transactionSubmitMultiSigned( } // There may not be a TxnSignature field. - if (stpTrans->isFieldPresent(sfTxnSignature)) + if (stTx->isFieldPresent(sfTxnSignature)) return rpcError(rpcSIGNING_MALFORMED); // The Fee field must be in XRP and greater than zero. - auto const fee = stpTrans->getFieldAmount(sfFee); + auto const fee = stTx->getFieldAmount(sfFee); if (!isLegalNet(fee)) { @@ -1298,12 +1297,12 @@ transactionSubmitMultiSigned( } // Verify that the Signers field is present. - if (!stpTrans->isFieldPresent(sfSigners)) + if (!stTx->isFieldPresent(sfSigners)) return RPC::missing_field_error("tx_json.Signers"); // If the Signers field is present the SField guarantees it to be an array. // Get a reference to the Signers array so we can verify and sort it. - auto& signers = stpTrans->peekFieldArray(sfSigners); + auto& signers = stTx->peekFieldArray(sfSigners); if (signers.empty()) return RPC::make_param_error("tx_json.Signers array may not be empty."); @@ -1330,7 +1329,7 @@ transactionSubmitMultiSigned( // Make sure the SerializedTransaction makes a legitimate Transaction. std::pair txn = - transactionConstructImpl(stpTrans, ledger->rules(), app); + transactionConstructImpl(stTx, ledger->rules(), app); if (!txn.second) return txn.first; diff --git a/src/xrpld/rpc/handlers/Simulate.cpp b/src/xrpld/rpc/handlers/Simulate.cpp index 6f3768c6a72..7caff0b78b7 100644 --- a/src/xrpld/rpc/handlers/Simulate.cpp +++ b/src/xrpld/rpc/handlers/Simulate.cpp @@ -33,7 +33,42 @@ namespace ripple { -std::optional +static Expected +getAutofillSequence(Json::Value tx_json, RPC::JsonContext& context) +{ + // autofill Sequence + bool const hasTicketSeq = tx_json.isMember(sfTicketSequence.jsonName); + auto const accountStr = tx_json[jss::Account]; + if (!accountStr.isString()) + { + // sanity check, should fail earlier + return Unexpected( + RPC::invalid_field_error("tx.Account")); // LCOV_EXCL_LINE + } + + auto const srcAddressID = + parseBase58(tx_json[jss::Account].asString()); + if (!srcAddressID.has_value()) + { + return Unexpected(RPC::make_error( + rpcSRC_ACT_MALFORMED, RPC::invalid_field_message("tx.Account"))); + } + + std::shared_ptr sle = context.app.openLedger().current()->read( + keylet::account(*srcAddressID)); + if (!hasTicketSeq && !sle) + { + JLOG(context.app.journal("Simulate").debug()) + << "Failed to find source account " + << "in current ledger: " << toBase58(*srcAddressID); + + return Unexpected(rpcError(rpcSRC_ACT_NOT_FOUND)); + } + + return hasTicketSeq ? 0 : context.app.getTxQ().nextQueuableSeq(sle).value(); +} + +static std::optional autofillTx(Json::Value& tx_json, RPC::JsonContext& context) { if (!tx_json.isMember(jss::Fee)) @@ -97,38 +132,10 @@ autofillTx(Json::Value& tx_json, RPC::JsonContext& context) 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()) - { - // sanity check, should fail earlier - return RPC::invalid_field_error("tx.Account"); // LCOV_EXCL_LINE - } - - auto const 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)); - if (!hasTicketSeq && !sle) - { - JLOG(context.app.journal("Simulate").debug()) - << "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(); + auto const seq = getAutofillSequence(tx_json, context); + if (!seq) + return seq.error(); + tx_json[sfSequence.jsonName] = *seq; } return std::nullopt; @@ -156,22 +163,25 @@ doSimulate(RPC::JsonContext& context) } } + // get JSON equivalent of transaction 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); + return RPC::make_param_error( + "Can only include one of `tx_blob` and `tx_json`."); } - auto const blob = context.params[jss::tx_blob]; - if (!blob.isString()) + + auto const tx_blob = context.params[jss::tx_blob]; + if (!tx_blob.isString()) { return RPC::invalid_field_error(jss::tx_blob); } - auto unHexed = strUnHex(blob.asString()); + auto unHexed = strUnHex(tx_blob.asString()); if (!unHexed || !unHexed->size()) return RPC::invalid_field_error(jss::tx_blob); + try { SerialIter sitTrans(makeSlice(*unHexed)); @@ -193,8 +203,8 @@ doSimulate(RPC::JsonContext& context) } else { - // neither `tx_blob` nor `tx_json` included - return rpcError(rpcINVALID_PARAMS); + return RPC::make_param_error( + "Neither `tx_blob` nor `tx_json` included."); } // basic sanity checks for transaction shape @@ -202,6 +212,7 @@ doSimulate(RPC::JsonContext& context) { return RPC::missing_field_error("tx.TransactionType"); } + if (!tx_json.isMember(jss::Account)) { return RPC::missing_field_error("tx.Account"); @@ -220,10 +231,10 @@ doSimulate(RPC::JsonContext& context) return jvResult; } - std::shared_ptr stpTrans; + std::shared_ptr stTx; try { - stpTrans = std::make_shared(std::move(parsed.object.value())); + stTx = std::make_shared(std::move(parsed.object.value())); } catch (std::exception& e) { @@ -234,7 +245,7 @@ doSimulate(RPC::JsonContext& context) } std::string reason; - auto tpTrans = std::make_shared(stpTrans, reason, context.app); + auto transaction = std::make_shared(stTx, reason, context.app); // Actually run the transaction through the transaction processor try @@ -244,7 +255,11 @@ doSimulate(RPC::JsonContext& context) // Process the transaction OpenView view = *context.app.openLedger().current(); auto const result = context.app.getTxQ().apply( - context.app, view, tpTrans->getSTransaction(), flags, context.j); + context.app, + view, + transaction->getSTransaction(), + flags, + context.j); jvResult[jss::applied] = result.applied; jvResult[jss::ledger_index] = view.seq(); @@ -253,19 +268,29 @@ doSimulate(RPC::JsonContext& context) context.params.get(jss::binary, false).asBool(); // Convert the TER to human-readable values - std::string sToken; - std::string sHuman; - transResultInfo(result.ter, sToken, sHuman); - - // Engine result - jvResult[jss::engine_result] = sToken; - jvResult[jss::engine_result_code] = result.ter; - jvResult[jss::engine_result_message] = sHuman; - if (sToken == "tesSUCCESS") + std::string token; + std::string message; + if (transResultInfo(result.ter, token, message)) + { + // Engine result + jvResult[jss::engine_result] = token; + jvResult[jss::engine_result_code] = result.ter; + jvResult[jss::engine_result_message] = message; + } + else { - static const std::string alternateSuccessMessage = + // shouldn't be hit + // LCOV_EXCL_START + jvResult[jss::engine_result] = "unknown"; + jvResult[jss::engine_result_code] = result.ter; + jvResult[jss::engine_result_message] = "unknown"; + // LCOV_EXCL_STOP + } + + if (token == "tesSUCCESS") + { + jvResult[jss::engine_result_message] = "The simulated transaction would have been applied."; - jvResult[jss::engine_result_message] = alternateSuccessMessage; } if (result.metadata) @@ -282,14 +307,15 @@ doSimulate(RPC::JsonContext& context) result.metadata->getJson(JsonOptions::none); } } + if (isBinaryOutput) { - auto const txBlob = stpTrans->getSerializer().getData(); + auto const txBlob = stTx->getSerializer().getData(); jvResult[jss::tx_blob] = strHex(makeSlice(txBlob)); } else { - jvResult[jss::tx_json] = tpTrans->getJson(JsonOptions::none); + jvResult[jss::tx_json] = transaction->getJson(JsonOptions::none); } return jvResult; diff --git a/src/xrpld/rpc/handlers/Submit.cpp b/src/xrpld/rpc/handlers/Submit.cpp index ecab0117f2f..0a3fe419238 100644 --- a/src/xrpld/rpc/handlers/Submit.cpp +++ b/src/xrpld/rpc/handlers/Submit.cpp @@ -83,11 +83,11 @@ doSubmit(RPC::JsonContext& context) SerialIter sitTrans(makeSlice(*ret)); - std::shared_ptr stpTrans; + std::shared_ptr stTx; try { - stpTrans = std::make_shared(std::ref(sitTrans)); + stTx = std::make_shared(std::ref(sitTrans)); } catch (std::exception& e) { @@ -101,11 +101,11 @@ doSubmit(RPC::JsonContext& context) if (!context.app.checkSigs()) forceValidity( context.app.getHashRouter(), - stpTrans->getTransactionID(), + stTx->getTransactionID(), Validity::SigGoodOnly); auto [validity, reason] = checkValidity( context.app.getHashRouter(), - *stpTrans, + *stTx, context.ledgerMaster.getCurrentLedger()->rules(), context.app.config()); if (validity != Validity::Valid) @@ -118,8 +118,8 @@ doSubmit(RPC::JsonContext& context) } std::string reason; - auto tpTrans = std::make_shared(stpTrans, reason, context.app); - if (tpTrans->getStatus() != NEW) + auto transaction = std::make_shared(stTx, reason, context.app); + if (transaction->getStatus() != NEW) { jvResult[jss::error] = "invalidTransaction"; jvResult[jss::error_exception] = "fails local checks: " + reason; @@ -132,7 +132,7 @@ doSubmit(RPC::JsonContext& context) auto const failType = getFailHard(context); context.netOps.processTransaction( - tpTrans, isUnlimited(context.role), true, failType); + transaction, isUnlimited(context.role), true, failType); } catch (std::exception& e) { @@ -144,22 +144,22 @@ doSubmit(RPC::JsonContext& context) try { - jvResult[jss::tx_json] = tpTrans->getJson(JsonOptions::none); + jvResult[jss::tx_json] = transaction->getJson(JsonOptions::none); jvResult[jss::tx_blob] = - strHex(tpTrans->getSTransaction()->getSerializer().peekData()); + strHex(transaction->getSTransaction()->getSerializer().peekData()); - if (temUNCERTAIN != tpTrans->getResult()) + if (temUNCERTAIN != transaction->getResult()) { std::string sToken; std::string sHuman; - transResultInfo(tpTrans->getResult(), sToken, sHuman); + transResultInfo(transaction->getResult(), sToken, sHuman); jvResult[jss::engine_result] = sToken; - jvResult[jss::engine_result_code] = tpTrans->getResult(); + jvResult[jss::engine_result_code] = transaction->getResult(); jvResult[jss::engine_result_message] = sHuman; - auto const submitResult = tpTrans->getSubmitResult(); + auto const submitResult = transaction->getSubmitResult(); jvResult[jss::accepted] = submitResult.any(); jvResult[jss::applied] = submitResult.applied; @@ -167,7 +167,7 @@ doSubmit(RPC::JsonContext& context) jvResult[jss::queued] = submitResult.queued; jvResult[jss::kept] = submitResult.kept; - if (auto currentLedgerState = tpTrans->getCurrentLedgerState()) + if (auto currentLedgerState = transaction->getCurrentLedgerState()) { jvResult[jss::account_sequence_next] = safe_cast(