Skip to content

Commit

Permalink
modify json rpc
Browse files Browse the repository at this point in the history
  • Loading branch information
yinyiqian1 committed Mar 3, 2025
1 parent 06fd21f commit 918488d
Show file tree
Hide file tree
Showing 5 changed files with 358 additions and 12 deletions.
27 changes: 23 additions & 4 deletions src/test/app/AccountPermission_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -418,10 +418,6 @@ class AccountPermission_test : public beast::unit_test::suite
env.fund(XRP(10000), alice, bob, carol);
env.close();

env(account_permission::accountPermissionSet(
alice, bob, {"CheckCreate"}));
env.close();

// add initial sequences and add sequence distance between alice and bob
for (int i = 0; i < 20; i++)
env(check::create(alice, carol, XRP(1)));
Expand Down Expand Up @@ -477,6 +473,29 @@ class AccountPermission_test : public beast::unit_test::suite
aliceSequence++;
BEAST_EXPECT(env.seq(alice) == aliceSequence);

// delegate sequence and ticket error should come before permission
// error, so that bad sequence or ticket are not consumed in transaction
// reset
env(check::create(bob, carol, XRP(1)),
onBehalfOf(alice),
delegateSequence(1),
ter(tefPAST_SEQ));
env.close();
env(check::create(bob, carol, XRP(1)),
onBehalfOf(alice),
delegateSequence(0),
delegateTicketSequence(1),
ter(tefNO_TICKET));
BEAST_EXPECT(env.seq(alice) == aliceSequence);
BEAST_EXPECT(env.seq(bob) == bobSequence);

// add permission
env(account_permission::accountPermissionSet(
alice, bob, {"CheckCreate"}));
env.close();
aliceSequence++;
BEAST_EXPECT(env.seq(alice) == aliceSequence);

// non existing delegating account
Account bad{"bad"};
env(check::create(bob, carol, XRP(1)),
Expand Down
44 changes: 44 additions & 0 deletions src/test/rpc/JSONRPC_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2084,6 +2084,50 @@ static constexpr TxnTestData txnTestArray[] = {
"Invalid field 'tx_json.DelegateTicketSequence'.",
"Invalid field 'tx_json.DelegateTicketSequence'."}}},

{"'OnBehalfOf' account needs to be well formed",
__LINE__,
R"({
"command": "doesnt_matter",
"account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
"secret": "masterpassphrase",
"tx_json": {
"Fee": 10,
"Account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
"Amount": "1000000000",
"Destination": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
"TransactionType": "Payment",
"Sequence": 10,
"OnBehalfOf": "NotAnAccount",
"DelegateSequence": 100
}
})",
{{"Invalid field 'tx_json.OnBehalfOf'.",
"Invalid field 'tx_json.OnBehalfOf'.",
"Invalid field 'tx_json.OnBehalfOf'.",
"Missing field 'tx_json.SigningPubKey'."}}},

{"'OnBehalfOf' account needs to exist",
__LINE__,
R"({
"command": "doesnt_matter",
"account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
"secret": "masterpassphrase",
"tx_json": {
"Fee": 10,
"Account": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
"Amount": "1000000000",
"Destination": "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
"TransactionType": "Payment",
"Sequence": 10,
"OnBehalfOf": "r9zN9x52FiCFAcicCLMQKbj1nxYhxJbbSy",
"DelegateSequence": 100
}
})",
{{"Source account not found.",
"Source account not found.",
"Source account not found.",
"Missing field 'tx_json.SigningPubKey'."}}},

{"If 'OnBehalfOf' is supplied, 'DelegateSequence' is auto filled for sign "
"and submit",
__LINE__,
Expand Down
234 changes: 234 additions & 0 deletions src/test/rpc/Simulate_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
//==============================================================================

#include <test/jtx.h>
#include <test/jtx/AccountPermission.h>
#include <test/jtx/Env.h>
#include <test/jtx/envconfig.h>
#include <test/jtx/ticket.h>
#include <xrpld/app/rdb/backend/SQLiteDatabase.h>
#include <xrpld/rpc/CTID.h>
#include <xrpl/protocol/ErrorCodes.h>
Expand Down Expand Up @@ -1074,6 +1076,236 @@ class Simulate_test : public beast::unit_test::suite
}
}

void
testOnBehalfOf()
{
testcase("OnBehalfOf");

using namespace jtx;
Env env{*this, envconfig([&](std::unique_ptr<Config> cfg) {
cfg->NETWORK_ID = 0;
return cfg;
})};

Account const alice{"alice"};
Account const bob{"bob"};
env.fund(XRP(10000), alice, bob);
env(account_permission::accountPermissionSet(alice, bob, {"Payment"}));
auto aliceTicket = env.seq(alice) + 1;
env(ticket::create(alice, 1));

{
auto validateOutput = [&](Json::Value const& resp,
Json::Value const& tx) {
auto result = resp[jss::result];
checkBasicReturnValidity(
result, tx, env.seq(bob), env.current()->fees().base);

Json::Value tx_json;
if (result.isMember(jss::tx_json))
{
tx_json = result[jss::tx_json];
}
else
{
auto const unHexed =
strUnHex(result[jss::tx_blob].asString());
SerialIter sitTrans(makeSlice(*unHexed));
tx_json = STObject(std::ref(sitTrans), sfGeneric)
.getJson(JsonOptions::none);
}

if (tx.isMember(jss::DelegateTicketSequence))
{
BEAST_EXPECT(
tx_json[jss::DelegateTicketSequence] ==
tx[jss::DelegateTicketSequence]);
BEAST_EXPECT(tx_json[jss::DelegateSequence] == 0);
}
else
{
BEAST_EXPECT(
tx_json[jss::DelegateSequence] == env.seq(alice));
}

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 const metadata = getJsonMetadata(result);

if (BEAST_EXPECT(
metadata.isMember(sfAffectedNodes.jsonName)))
{
auto nAffectedNodes =
metadata[sfAffectedNodes.jsonName].size();
BEAST_EXPECT(nAffectedNodes >= 2);

auto nModifiedAccount = 0;
for (Json::Value::UInt i = 0; i < nAffectedNodes; i++)
{
auto node = metadata[sfAffectedNodes.jsonName][i];
if (node.isMember(sfModifiedNode.jsonName))
{
auto modifiedNode = node[sfModifiedNode];
if (modifiedNode[sfLedgerEntryType] ==
"AccountRoot")
{
nModifiedAccount++;
}
}
}
BEAST_EXPECT(nModifiedAccount == 2);
}
BEAST_EXPECT(metadata[sfTransactionIndex.jsonName] == 0);
BEAST_EXPECT(
metadata[sfTransactionResult.jsonName] == "tesSUCCESS");
}
};

Json::Value tx;
tx[jss::TransactionType] = jss::Payment;
tx[jss::Account] = bob.human();
tx[jss::Destination] = bob.human();
tx[jss::Amount] = 1;
tx[sfSigningPubKey] = "";
tx[sfTxnSignature] = "";
tx[sfSequence] = env.seq(bob);
tx[sfFee] = env.current()->fees().base.jsonClipped().asString();
tx[sfOnBehalfOf] = alice.human();

// test auto fill DelegateSequence
testTx(env, tx, validateOutput);

// test specifying DelegateSequence
tx[sfDelegateSequence] = env.seq(alice);
testTx(env, tx, validateOutput);

Json::Value tx2;
tx2[jss::TransactionType] = jss::Payment;
tx2[jss::Account] = bob.human();
tx2[jss::Destination] = bob.human();
tx2[jss::Amount] = 1;
tx2[sfSigningPubKey] = "";
tx2[sfTxnSignature] = "";
tx2[sfSequence] = env.seq(bob);
tx2[sfFee] = env.current()->fees().base.jsonClipped().asString();
tx2[sfOnBehalfOf] = alice.human();

// test auto fill DelegateSequence when DelegateTicketSequence is
// specified
tx2[sfDelegateTicketSequence] = aliceTicket;
testTx(env, tx2, validateOutput);

// test setting DelegateSequence to 0 when DelegateTicketSequence is
// specified
tx2[sfDelegateSequence] = 0;
testTx(env, tx2, validateOutput);
}
}

void
testOnBehalfOfParamErrors()
{
testcase("OnBehalfOf parameter errors");

using namespace jtx;
Env env(*this);
Account const alice{"alice"};
Account const bob{"bob"};
env.fund(XRP(10000), alice, bob);
env(account_permission::accountPermissionSet(alice, bob, {"Payment"}));
env.close();
Account const nonExist{"nonExist"};

{
// test specifying DelegateSequence without OnBehalfOf
Json::Value params;
Json::Value tx_json = Json::objectValue;
tx_json[jss::TransactionType] = jss::Payment;
tx_json[jss::Account] = bob.human();
tx_json[jss::Destination] = bob.human();
tx_json[jss::Amount] = 1;
tx_json[jss::DelegateSequence] = env.seq(alice);
params[jss::tx_json] = tx_json;

auto const resp = env.rpc("json", "simulate", to_string(params));
BEAST_EXPECT(
resp[jss::result][jss::error_message] ==
"Invalid field 'tx_json.DelegateSequence'.");
}
{
// test specifying DelegateTicketSequence without OnBehalfOf
Json::Value params;
Json::Value tx_json = Json::objectValue;
tx_json[jss::TransactionType] = jss::Payment;
tx_json[jss::Account] = alice.human();
tx_json[jss::Destination] = bob.human();
tx_json[jss::Amount] = 1;
tx_json[jss::DelegateTicketSequence] = env.seq(alice);
params[jss::tx_json] = tx_json;

auto const resp = env.rpc("json", "simulate", to_string(params));
BEAST_EXPECT(
resp[jss::result][jss::error_message] ==
"Invalid field 'tx_json.DelegateTicketSequence'.");
}
{
// test bad on behalf of account which is not string
Json::Value params;
Json::Value tx_json = Json::objectValue;
tx_json[jss::TransactionType] = jss::Payment;
tx_json[jss::Account] = bob.human();
tx_json[jss::Destination] = bob.human();
tx_json[jss::Amount] = 1;
tx_json[jss::OnBehalfOf] = 1;
params[jss::tx_json] = tx_json;

auto const resp = env.rpc("json", "simulate", to_string(params));
BEAST_EXPECT(
resp[jss::result][jss::error_message] ==
"Invalid field 'tx_json.OnBehalfOf'.");
}
{
// test bad on behalf of account
Json::Value params;
Json::Value tx_json = Json::objectValue;
tx_json[jss::TransactionType] = jss::Payment;
tx_json[jss::Account] = bob.human();
tx_json[jss::Destination] = bob.human();
tx_json[jss::Amount] = 1;
tx_json[jss::OnBehalfOf] = "badAccount";
params[jss::tx_json] = tx_json;

auto const resp = env.rpc("json", "simulate", to_string(params));
BEAST_EXPECT(
resp[jss::result][jss::error_message] ==
"Invalid field 'tx_json.OnBehalfOf'.");
}
{
// test non exist on behalf of account
Json::Value params;
Json::Value tx_json = Json::objectValue;
tx_json[jss::TransactionType] = jss::Payment;
tx_json[jss::Account] = bob.human();
tx_json[jss::Destination] = bob.human();
tx_json[jss::Amount] = 1;
tx_json[jss::OnBehalfOf] = nonExist.human();
params[jss::tx_json] = tx_json;

auto const resp = env.rpc("json", "simulate", to_string(params));
BEAST_EXPECT(
resp[jss::result][jss::error_message] ==
"Source account not found.");
}
}

public:
void
run() override
Expand All @@ -1088,6 +1320,8 @@ class Simulate_test : public beast::unit_test::suite
testMultisignedBadPubKey();
testDeleteExpiredCredentials();
testSuccessfulTransactionNetworkID();
testOnBehalfOf();
testOnBehalfOfParamErrors();
}
};

Expand Down
8 changes: 0 additions & 8 deletions src/xrpld/rpc/detail/TransactionSign.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -511,14 +511,6 @@ transactionPreProcessImpl(
{
bool const hasDelegateTicketSeq =
tx_json.isMember(sfDelegateTicketSequence.jsonName);
if (!hasDelegateTicketSeq && !delegatingAccountSle)
{
JLOG(j.debug())
<< "transactionSign: Failed to find on behalf of account "
<< "in current ledger: " << toBase58(delegatingAccount);

return rpcError(rpcSRC_ACT_NOT_FOUND);
}
tx_json[jss::DelegateSequence] = hasDelegateTicketSeq
? 0
: app.getTxQ().nextQueuableSeq(delegatingAccountSle).value();
Expand Down
Loading

0 comments on commit 918488d

Please sign in to comment.