Test for 2 types of bad DomainIDs in PermissionedDomainSet.

Also, test sorting of permissions.

src/test/app/PermissionedDomains_test.cpp

@@ -175,6 +175,34 @@ class PermissionedDomains_test : public beast::unit_test::suite
         env(pd::setTx(account, credentialsDup, domain),
             fee(setFee),
             ter(temMALFORMED));
+
+        // Have equal issuers but different credentials and make sure they
+        // sort correctly.
+        {
+            pd::Credentials const credentialsSame{
+                {alice2, pd::toBlob("credential3")},
+                {alice3, pd::toBlob("credential2")},
+                {alice2, pd::toBlob("credential9")},
+                {alice5, pd::toBlob("credential4")},
+                {alice2, pd::toBlob("credential6")},
+            };
+            BEAST_EXPECT(
+                credentialsSame != *pd::sortCredentials(credentialsSame));
+            env(pd::setTx(account, credentialsSame, domain), fee(setFee));
+
+            uint256 d;
+            if (domain)
+                d = *domain;
+            else
+                d = pd::getNewDomain(env.meta());
+            env.close();
+            auto objects = pd::getObjects(account, env);
+            auto const fromObject = pd::credentialsFromJson(objects[d]);
+            auto const sortedCreds = *pd::sortCredentials(credentialsSame);
+            BEAST_EXPECT(
+                pd::credentialsFromJson(objects[d]) ==
+                *pd::sortCredentials(credentialsSame));
+        }
     }
 
     // Test PermissionedDomainSet
@@ -238,24 +266,10 @@ class PermissionedDomains_test : public beast::unit_test::suite
         {
             BEAST_EXPECT(
                 credentials10.size() == PermissionedDomainSet::PD_ARRAY_MAX);
-            BEAST_EXPECT(credentials10 != pd::sortCredentials(credentials10));
+            BEAST_EXPECT(credentials10 != *pd::sortCredentials(credentials10));
             env(pd::setTx(alice, credentials10), fee(setFee));
             auto tx = env.tx()->getJson(JsonOptions::none);
-            auto meta = env.meta()->getJson(JsonOptions::none);
-            Json::Value a(Json::arrayValue);
-            a = meta["AffectedNodes"];
-
-            for (auto const& node : a)
-            {
-                if (!node.isMember("CreatedNode") ||
-                    node["CreatedNode"]["LedgerEntryType"] !=
-                        "PermissionedDomain")
-                {
-                    continue;
-                }
-                std::ignore = domain2.parseHex(
-                    node["CreatedNode"]["LedgerIndex"].asString());
-            }
+            domain2 = pd::getNewDomain(env.meta());
             auto objects = pd::getObjects(alice, env);
             auto object = objects[domain2];
             BEAST_EXPECT(
@@ -285,6 +299,12 @@ class PermissionedDomains_test : public beast::unit_test::suite
         env(pd::setTx(alice2, credentials1, domain2),
             ter(temINVALID_ACCOUNT_ID));
 
+        // Update a uint256(0) domain
+        env(pd::setTx(alice, credentials1, uint256(0)), ter(temMALFORMED));
+
+        // Update non-existent domain
+        env(pd::setTx(alice, credentials1, uint256(75)), ter(tecNO_ENTRY));
+
         // Test bad data when creating a domain.
         testBadData(alice, env);
         // Test bad data when updating a domain.

src/test/jtx/PermissionedDomains.h

@@ -21,6 +21,7 @@
 #define RIPPLE_TEST_JTX_PERMISSIONEDDOMAINS_H_INCLUDED
 
 #include <test/jtx.h>
+#include <optional>
 
 namespace ripple {
 namespace test {
@@ -63,13 +64,17 @@ Credentials
 credentialsFromJson(Json::Value const& object);
 
 // Sort credentials the same way as PermissionedDomainSet
-Credentials
+std::optional<Credentials>
 sortCredentials(Credentials const& input);
 
 // Get account_info
 Json::Value
 ownerInfo(Account const& account, Env& env);
 
+// Get newly created domain from transaction metadata.
+uint256
+getNewDomain(std::shared_ptr<STObject const> const& meta);
+
 }  // namespace pd
 }  // namespace jtx
 }  // namespace test

src/test/jtx/impl/PermissionedDomains.cpp

@@ -18,6 +18,7 @@
 //==============================================================================
 
 #include <test/jtx/PermissionedDomains.h>
+#include <exception>
 
 namespace ripple {
 namespace test {
@@ -120,18 +121,56 @@ credentialsFromJson(Json::Value const& object)
     return ret;
 }
 
-// Sort credentials the same way as PermissionedDomainSet
-Credentials
+// Sort credentials the same way as PermissionedDomainSet. None can
+// be identical.
+std::optional<Credentials>
 sortCredentials(Credentials const& input)
 {
-    Credentials ret = input;
-    std::sort(
-        ret.begin(),
-        ret.end(),
-        [](Credential const& left, Credential const& right) -> bool {
-            return left.first < right.first;
-        });
-    return ret;
+    try
+    {
+        Credentials ret = input;
+        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;
+                    if (left.second == right.second)
+                        throw std::runtime_error("duplicate");
+                }
+                return false;
+                /*
+                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;
+                    }
+                    if (left.getFieldVL(sfCredentialType) ==
+                        right.getFieldVL(sfCredentialType))
+                    {
+                        throw std::runtime_error("duplicate credentials");
+                    }
+                    return false;
+                }
+                return false;
+                return left.first < right.first;
+                 */
+            });
+        return ret;
+    }
+    catch (...)
+    {
+        std::cerr << "wtf\n";
+        return std::nullopt;
+    }
 }
 
 // Get account_info
@@ -145,6 +184,28 @@ ownerInfo(Account const& account, Env& env)
         "json", "account_info", to_string(params))["result"]["account_data"];
 }
 
+uint256
+getNewDomain(std::shared_ptr<STObject const> const& meta)
+{
+    uint256 ret;
+    auto metaJson = meta->getJson(JsonOptions::none);
+    Json::Value a(Json::arrayValue);
+    a = metaJson["AffectedNodes"];
+
+    for (auto const& node : a)
+    {
+        if (!node.isMember("CreatedNode") ||
+            node["CreatedNode"]["LedgerEntryType"] != "PermissionedDomain")
+        {
+            continue;
+        }
+        std::ignore =
+            ret.parseHex(node["CreatedNode"]["LedgerIndex"].asString());
+    }
+
+    return ret;
+}
+
 }  // namespace pd
 }  // namespace jtx
 }  // namespace test

src/xrpld/app/tx/detail/PermissionedDomainSet.cpp

@@ -117,7 +117,6 @@ PermissionedDomainSet::doApply()
                     {
                         throw std::runtime_error("duplicate credentials");
                     }
-                    return false;
                 }
                 return false;
             });