Skip to content

Commit

Permalink
RXI-1265 review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@oleks-rip
oleks-rip committed Nov 7, 2024
1 parent 7a083ec commit 5ac8ef1
Show file tree
Hide file tree
Showing 6 changed files with 158 additions and 91 deletions.
41 changes: 7 additions & 34 deletions src/test/app/PermissionedDomains_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
//==============================================================================

#include <test/jtx.h>
#include <test/jtx/PermissionedDomains.h>
#include <xrpld/app/tx/detail/PermissionedDomainSet.h>
#include <xrpld/ledger/ApplyViewImpl.h>
#include <xrpl/basics/Blob.h>
Expand Down Expand Up @@ -410,9 +409,13 @@ class PermissionedDomains_test : public beast::unit_test::suite
BEAST_EXPECT(pd::getObjects(alice, env).size() == 0);
env.close();

auto const baseFee = env.current()->fees().base.drops();

// Pay alice almost enough to make the reserve.
env(pay(env.master, alice, incReserve + drops(19)));
BEAST_EXPECT(env.balance(alice) == acctReserve + incReserve + drops(9));
env(pay(env.master, alice, incReserve + drops(2 * baseFee) - drops(1)));
BEAST_EXPECT(
env.balance(alice) ==
acctReserve + incReserve + drops(baseFee) - drops(1));
env.close();

// alice still does not have enough XRP for the reserve.
Expand All @@ -421,43 +424,13 @@ class PermissionedDomains_test : public beast::unit_test::suite
BEAST_EXPECT(pd::ownerInfo(alice, env)["OwnerCount"].asUInt() == 0);

// Pay alice enough to make the reserve.
env(pay(env.master, alice, drops(11)));
env(pay(env.master, alice, drops(baseFee) + drops(1)));
env.close();

// Now alice can create a PermissionedDomain.
env(pd::setTx(alice, credentials));
env.close();
BEAST_EXPECT(pd::ownerInfo(alice, env)["OwnerCount"].asUInt() == 1);

/*
env(did::setValid(alice), ter(tecINSUFFICIENT_RESERVE));
env.close();
BEAST_EXPECT(ownerCount(env, alice) == 0);
// Pay alice almost enough to make the reserve for a DID.
env(pay(env.master, alice, incReserve + drops(19)));
BEAST_EXPECT(env.balance(alice) == acctReserve + incReserve +
drops(9)); env.close();
// alice still does not have enough XRP for the reserve of a
DID. env(did::setValid(alice), ter(tecINSUFFICIENT_RESERVE));
env.close();
BEAST_EXPECT(ownerCount(env, alice) == 0);
// Pay alice enough to make the reserve for a DID.
env(pay(env.master, alice, drops(11)));
env.close();
// Now alice can create a DID.
env(did::setValid(alice));
env.close();
BEAST_EXPECT(ownerCount(env, alice) == 1);
// alice deletes her DID.
env(did::del(alice));
BEAST_EXPECT(ownerCount(env, alice) == 0);
env.close();
*/
}

public:
Expand Down
1 change: 1 addition & 0 deletions src/test/jtx.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <test/jtx/Env.h>
#include <test/jtx/Env_ss.h>
#include <test/jtx/JTx.h>
#include <test/jtx/PermissionedDomains.h>
#include <test/jtx/TestHelpers.h>
#include <test/jtx/account_txn_id.h>
#include <test/jtx/acctdelete.h>
Expand Down
30 changes: 4 additions & 26 deletions src/test/jtx/impl/PermissionedDomains.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,32 +128,10 @@ credentialsFromJson(Json::Value const& object)
Credentials
sortCredentials(Credentials const& input)
{
Credentials ret = input;

std::set<Credential> cSet;
for (auto const& c : ret)
cSet.insert(c);
if (ret.size() > cSet.size())
{
ret = Credentials();
for (auto const& c : cSet)
ret.push_back(c);
}

std::sort(
ret.begin(),
ret.end(),
[](Credential const& left, Credential const& right) -> bool {
if (left.first < right.first)
return true;
if (left.first == right.first)
{
if (left.second < right.second)
return true;
}
return false;
});
return ret;
std::set<Credential> credentialsSet;
for (auto const& c : input)
credentialsSet.insert(c);
return {credentialsSet.begin(), credentialsSet.end()};
}

// Get account_info
Expand Down
90 changes: 90 additions & 0 deletions src/test/rpc/LedgerRPC_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2928,6 +2928,95 @@ class LedgerRPC_test : public beast::unit_test::suite
}
}

void
testLedgerEntryPermissionedDomain()
{
testcase("ledger_entry PermissionedDomain");

using namespace test::jtx;

Env env(*this, supported_amendments() | featurePermissionedDomains);
Account const issuer{"issuer"};
Account const alice{"alice"};
Account const bob{"bob"};

env.fund(XRP(5000), issuer, alice, bob);
env.close();

auto const seq = env.seq(alice);
env(pd::setTx(alice, {{alice, pd::toBlob("first credential")}}));
env.close();
auto const objects = pd::getObjects(alice, env);
BEAST_EXPECT(objects.size() == 1);
if (objects.empty())
return;
// env(pd::deleteTx(alice, domain));

{
// Succeed
Json::Value params;
params[jss::ledger_index] = jss::validated;
params[jss::permissioned_domain][jss::account] = alice.human();
params[jss::permissioned_domain][jss::seq] = seq;
auto jv = env.rpc("json", "ledger_entry", to_string(params));
BEAST_EXPECT(
jv.isObject() && jv.isMember(jss::result) &&
!jv[jss::result].isMember(jss::error) &&
jv[jss::result].isMember(jss::node) &&
jv[jss::result][jss::node].isMember(
sfLedgerEntryType.jsonName) &&
jv[jss::result][jss::node][sfLedgerEntryType.jsonName] ==
jss::PermissionedDomain);

std::string const pdIdx = jv[jss::result][jss::index].asString();
BEAST_EXPECT(
strHex(keylet::permissionedDomain(alice, seq).key) == pdIdx);

params.clear();
params[jss::ledger_index] = jss::validated;
params[jss::permissioned_domain] = pdIdx;
jv = env.rpc("json", "ledger_entry", to_string(params));
BEAST_EXPECT(
jv.isObject() && jv.isMember(jss::result) &&
!jv[jss::result].isMember(jss::error) &&
jv[jss::result].isMember(jss::node) &&
jv[jss::result][jss::node].isMember(
sfLedgerEntryType.jsonName) &&
jv[jss::result][jss::node][sfLedgerEntryType.jsonName] ==
jss::PermissionedDomain);
}

{
// Fail, invalid account
Json::Value params;
params[jss::ledger_index] = jss::validated;
params[jss::permissioned_domain][jss::account] = 1;
params[jss::permissioned_domain][jss::seq] = seq;
auto const jrr = env.rpc("json", "ledger_entry", to_string(params));
checkErrorValue(jrr[jss::result], "malformedRequest", "");
}

{
// Fail, no account
Json::Value params;
params[jss::ledger_index] = jss::validated;
params[jss::permissioned_domain][jss::account] = "";
params[jss::permissioned_domain][jss::seq] = seq;
auto const jrr = env.rpc("json", "ledger_entry", to_string(params));
checkErrorValue(jrr[jss::result], "malformedRequest", "");
}

{
// Fail, invalid sequence
Json::Value params;
params[jss::ledger_index] = jss::validated;
params[jss::permissioned_domain][jss::account] = alice.human();
params[jss::permissioned_domain][jss::seq] = "12g";
auto const jrr = env.rpc("json", "ledger_entry", to_string(params));
checkErrorValue(jrr[jss::result], "malformedRequest", "");
}
}

public:
void
run() override
Expand Down Expand Up @@ -2958,6 +3047,7 @@ class LedgerRPC_test : public beast::unit_test::suite
testInvalidOracleLedgerEntry();
testOracleLedgerEntry();
testLedgerEntryMPT();
testLedgerEntryPermissionedDomain();

forAllApiVersions(std::bind_front(
&LedgerRPC_test::testLedgerEntryInvalidParams, this));
Expand Down
57 changes: 26 additions & 31 deletions src/xrpld/app/tx/detail/PermissionedDomainSet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ PermissionedDomainSet::preflight(PreflightContext const& ctx)
{
return temMALFORMED;
}
if (credential.getFieldVL(sfCredentialType).empty())
auto sz = credential.getFieldVL(sfCredentialType).size();
if (!sz || sz > 64)
return temMALFORMED;
}

Expand Down Expand Up @@ -94,43 +95,34 @@ PermissionedDomainSet::doApply()
// for either new domain or updating existing.
// Silently remove duplicates.
auto updateSle = [this](std::shared_ptr<STLedgerEntry> const& sle) {
auto credentials = ctx_.tx.getFieldArray(sfAcceptedCredentials);
std::map<uint256, STObject> hashed;
for (auto const& c : credentials)
hashed.insert({c.getHash(HashPrefix::transactionID), c});
if (credentials.size() > hashed.size())
{
credentials = STArray();
for (auto const& e : hashed)
credentials.push_back(e.second);
}
auto const& credentials = ctx_.tx.getFieldArray(sfAcceptedCredentials);

credentials.sort(
[](STObject const& left, STObject const& right) -> bool {
if (left.getAccountID(sfIssuer) < right.getAccountID(sfIssuer))
return true;
if (left.getAccountID(sfIssuer) == right.getAccountID(sfIssuer))
{
if (left.getFieldVL(sfCredentialType) <
right.getFieldVL(sfCredentialType))
{
return true;
}
}
return false;
});
sle->setFieldArray(sfAcceptedCredentials, credentials);
std::map<
std::pair<AccountID, Blob>,
std::reference_wrapper<STObject const>>
credMap;
for (auto const& c : credentials)
credMap.insert(
{{c.getAccountID(sfIssuer), c.getFieldVL(sfCredentialType)},
c});

STArray credSorted;
credSorted.reserve(credMap.size());
for (auto const& [k, v] : credMap)
credSorted.push_back(v);
sle->setFieldArray(sfAcceptedCredentials, credSorted);
};

if (ctx_.tx.isFieldPresent(sfDomainID))
{
// Modify existing permissioned domain.
auto sleUpdate = view().peek(
auto slePd = view().peek(
keylet::permissionedDomain(ctx_.tx.getFieldH256(sfDomainID)));
// It should already be checked in preclaim().
assert(sleUpdate);
updateSle(sleUpdate);
view().update(sleUpdate);
if (!slePd)
return tefINTERNAL;

updateSle(slePd);
view().update(slePd);
}
else
{
Expand All @@ -145,6 +137,9 @@ PermissionedDomainSet::doApply()
Keylet const pdKeylet = keylet::permissionedDomain(
account_, ctx_.tx.getFieldU32(sfSequence));
auto slePd = std::make_shared<SLE>(pdKeylet);
if (!slePd)
return tefINTERNAL;

slePd->setAccountID(sfOwner, account_);
slePd->setFieldU32(sfSequence, ctx_.tx.getFieldU32(sfSequence));
updateSle(slePd);
Expand Down
30 changes: 30 additions & 0 deletions src/xrpld/rpc/handlers/LedgerEntry.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,36 @@ doLedgerEntry(RPC::JsonContext& context)
}
}
}
else if (context.params.isMember(jss::permissioned_domain))
{
uNodeIndex = beast::zero;
auto const& pd = context.params[jss::permissioned_domain];
if (pd.isString())
{
if (!uNodeIndex.parseHex(pd.asString()))
{
uNodeIndex = beast::zero;
jvResult[jss::error] = "malformedRequest";
}
}
else if (
!pd.isObject() || !pd.isMember(jss::account) ||
!pd[jss::account].isString() || !pd.isMember(jss::seq) ||
(!pd[jss::seq].isInt() && !pd[jss::seq].isUInt()))
{
jvResult[jss::error] = "malformedRequest";
}
else
{
auto const account =
parseBase58<AccountID>(pd[jss::account].asString());
unsigned const seq = pd[jss::seq].asUInt();
if (account)
uNodeIndex = keylet::permissionedDomain(*account, seq).key;
else
jvResult[jss::error] = "malformedRequest";
}
}
else
{
if (context.params.isMember("params") &&
Expand Down

0 comments on commit 5ac8ef1

Please sign in to comment.