diff --git a/src/test/app/PermissionedDomains_test.cpp b/src/test/app/PermissionedDomains_test.cpp index 03dd6e98eb7..126e47aef9a 100644 --- a/src/test/app/PermissionedDomains_test.cpp +++ b/src/test/app/PermissionedDomains_test.cpp @@ -18,7 +18,6 @@ //============================================================================== #include -#include #include #include #include @@ -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. @@ -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: diff --git a/src/test/jtx.h b/src/test/jtx.h index b7b9a9fa05c..d9ef285a61d 100644 --- a/src/test/jtx.h +++ b/src/test/jtx.h @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include diff --git a/src/test/jtx/impl/PermissionedDomains.cpp b/src/test/jtx/impl/PermissionedDomains.cpp index 632efb7e485..7e7eb964415 100644 --- a/src/test/jtx/impl/PermissionedDomains.cpp +++ b/src/test/jtx/impl/PermissionedDomains.cpp @@ -128,32 +128,10 @@ credentialsFromJson(Json::Value const& object) Credentials sortCredentials(Credentials const& input) { - Credentials ret = input; - - std::set 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 credentialsSet; + for (auto const& c : input) + credentialsSet.insert(c); + return {credentialsSet.begin(), credentialsSet.end()}; } // Get account_info diff --git a/src/test/rpc/LedgerRPC_test.cpp b/src/test/rpc/LedgerRPC_test.cpp index c5e10198c49..28390e774b6 100644 --- a/src/test/rpc/LedgerRPC_test.cpp +++ b/src/test/rpc/LedgerRPC_test.cpp @@ -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 @@ -2958,6 +3047,7 @@ class LedgerRPC_test : public beast::unit_test::suite testInvalidOracleLedgerEntry(); testOracleLedgerEntry(); testLedgerEntryMPT(); + testLedgerEntryPermissionedDomain(); forAllApiVersions(std::bind_front( &LedgerRPC_test::testLedgerEntryInvalidParams, this)); diff --git a/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp index 7d296599cbc..f64cef0a00c 100644 --- a/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp +++ b/src/xrpld/app/tx/detail/PermissionedDomainSet.cpp @@ -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; } @@ -94,43 +95,34 @@ PermissionedDomainSet::doApply() // for either new domain or updating existing. // Silently remove duplicates. auto updateSle = [this](std::shared_ptr const& sle) { - auto credentials = ctx_.tx.getFieldArray(sfAcceptedCredentials); - std::map 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, + std::reference_wrapper> + 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 { @@ -145,6 +137,9 @@ PermissionedDomainSet::doApply() Keylet const pdKeylet = keylet::permissionedDomain( account_, ctx_.tx.getFieldU32(sfSequence)); auto slePd = std::make_shared(pdKeylet); + if (!slePd) + return tefINTERNAL; + slePd->setAccountID(sfOwner, account_); slePd->setFieldU32(sfSequence, ctx_.tx.getFieldU32(sfSequence)); updateSle(slePd); diff --git a/src/xrpld/rpc/handlers/LedgerEntry.cpp b/src/xrpld/rpc/handlers/LedgerEntry.cpp index 6a3b7a48686..1c210d9020a 100644 --- a/src/xrpld/rpc/handlers/LedgerEntry.cpp +++ b/src/xrpld/rpc/handlers/LedgerEntry.cpp @@ -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(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") &&