Skip to content

Commit

Permalink
more fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@mvadari
mvadari committed Jan 10, 2025
1 parent 301868d commit 0bbf589
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 110 deletions.
36 changes: 15 additions & 21 deletions src/test/rpc/Simulate_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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");
Expand All @@ -737,15 +742,15 @@ 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] = "";
tx[sfSequence] = env.seq(env.master);
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
}
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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);
Expand All @@ -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)))
{
Expand Down
35 changes: 17 additions & 18 deletions src/xrpld/rpc/detail/TransactionSign.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ transactionPreProcessImpl(
return err;
}

std::shared_ptr<STTx> stpTrans;
std::shared_ptr<STTx> stTx;
try
{
// If we're generating a multi-signature the SigningPubKey must be
Expand All @@ -549,7 +549,7 @@ transactionPreProcessImpl(
sfSigningPubKey,
signingArgs.isMultiSigning() ? Slice(nullptr, 0) : pk.slice());

stpTrans = std::make_shared<STTx>(std::move(parsed.object.value()));
stTx = std::make_shared<STTx>(std::move(parsed.object.value()));
}
catch (STObject::FieldErr& err)
{
Expand All @@ -563,30 +563,29 @@ 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());

signingArgs.moveMultiSignature(std::move(multisig));
}
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<Json::Value, Transaction::pointer>
transactionConstructImpl(
std::shared_ptr<STTx const> const& stpTrans,
std::shared_ptr<STTx const> const& stTx,
Rules const& rules,
Application& app)
{
Expand All @@ -596,7 +595,7 @@ transactionConstructImpl(
Transaction::pointer tpTrans;
{
std::string reason;
tpTrans = std::make_shared<Transaction>(stpTrans, reason, app);
tpTrans = std::make_shared<Transaction>(stTx, reason, app);
if (tpTrans->getStatus() != NEW)
{
ret.first = RPC::make_error(
Expand Down Expand Up @@ -1228,7 +1227,7 @@ transactionSubmitMultiSigned(
}

// Grind through the JSON in tx_json to produce a STTx.
std::shared_ptr<STTx> stpTrans;
std::shared_ptr<STTx> stTx;
{
STParsedJSONObject parsedTx_json("tx_json", tx_json);
if (!parsedTx_json.object)
Expand All @@ -1241,7 +1240,7 @@ transactionSubmitMultiSigned(
}
try
{
stpTrans =
stTx =
std::make_shared<STTx>(std::move(parsedTx_json.object.value()));
}
catch (STObject::FieldErr& err)
Expand All @@ -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);
}

Expand All @@ -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
Expand All @@ -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))
{
Expand All @@ -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.");
Expand All @@ -1330,7 +1329,7 @@ transactionSubmitMultiSigned(

// Make sure the SerializedTransaction makes a legitimate Transaction.
std::pair<Json::Value, Transaction::pointer> txn =
transactionConstructImpl(stpTrans, ledger->rules(), app);
transactionConstructImpl(stTx, ledger->rules(), app);

if (!txn.second)
return txn.first;
Expand Down
Loading

0 comments on commit 0bbf589

Please sign in to comment.