Permissioned Domains (XLS-80d)

src/test/app/PermissionedDomains_test.cpp

@@ -165,16 +165,32 @@ class PermissionedDomains_test : public beast::unit_test::suite
             .removeMember("Issuer");
         env(txJsonMutable, fee(setFee), ter(temMALFORMED));
 
-        // Make 2 identical credentials.
-        pd::Credentials const credentialsDup{
-            {alice2, pd::toBlob("credential1")},
-            {alice3, pd::toBlob("credential2")},
-            {alice2, pd::toBlob("credential1")},
-            {alice5, pd::toBlob("credential4")},
-        };
-        env(pd::setTx(account, credentialsDup, domain),
-            fee(setFee),
-            ter(temMALFORMED));
+        // Make 2 identical credentials. The duplicate should be silently
+        // removed.
+        {
+            pd::Credentials const credentialsDup{
+                {alice7, pd::toBlob("credential6")},
+                {alice2, pd::toBlob("credential1")},
+                {alice3, pd::toBlob("credential2")},
+                {alice2, pd::toBlob("credential1")},
+                {alice5, pd::toBlob("credential4")},
+            };
+            BEAST_EXPECT(pd::sortCredentials(credentialsDup).size() == 4);
+            env(pd::setTx(account, credentialsDup, 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(credentialsDup);
+            BEAST_EXPECT(
+                pd::credentialsFromJson(objects[d]) ==
+                pd::sortCredentials(credentialsDup));
+        }
 
         // Have equal issuers but different credentials and make sure they
         // sort correctly.
@@ -187,7 +203,7 @@ class PermissionedDomains_test : public beast::unit_test::suite
                 {alice2, pd::toBlob("credential6")},
             };
             BEAST_EXPECT(
-                credentialsSame != *pd::sortCredentials(credentialsSame));
+                credentialsSame != pd::sortCredentials(credentialsSame));
             env(pd::setTx(account, credentialsSame, domain), fee(setFee));
 
             uint256 d;
@@ -198,10 +214,10 @@ class PermissionedDomains_test : public beast::unit_test::suite
             env.close();
             auto objects = pd::getObjects(account, env);
             auto const fromObject = pd::credentialsFromJson(objects[d]);
-            auto const sortedCreds = *pd::sortCredentials(credentialsSame);
+            auto const sortedCreds = pd::sortCredentials(credentialsSame);
             BEAST_EXPECT(
                 pd::credentialsFromJson(objects[d]) ==
-                *pd::sortCredentials(credentialsSame));
+                pd::sortCredentials(credentialsSame));
         }
     }
 
@@ -266,7 +282,7 @@ 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);
             domain2 = pd::getNewDomain(env.meta());

src/test/jtx/PermissionedDomains.h

@@ -21,7 +21,6 @@
 #define RIPPLE_TEST_JTX_PERMISSIONEDDOMAINS_H_INCLUDED
 
 #include <test/jtx.h>
-#include <optional>
 
 namespace ripple {
 namespace test {
@@ -64,7 +63,7 @@ Credentials
 credentialsFromJson(Json::Value const& object);
 
 // Sort credentials the same way as PermissionedDomainSet
-std::optional<Credentials>
+Credentials
 sortCredentials(Credentials const& input);
 
 // Get account_info

src/test/jtx/impl/PermissionedDomains.cpp

@@ -121,56 +121,37 @@ credentialsFromJson(Json::Value const& object)
     return ret;
 }
 
-// Sort credentials the same way as PermissionedDomainSet. None can
-// be identical.
-std::optional<Credentials>
+// Sort credentials the same way as PermissionedDomainSet. Silently
+// remove duplicates.
+Credentials
 sortCredentials(Credentials const& input)
 {
-    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 (...)
+    Credentials ret = input;
+
+    std::set<Credential> cSet;
+    for (auto const& c : ret)
+        cSet.insert(c);
+    if (ret.size() > cSet.size())
     {
-        std::cerr << "wtf\n";
-        return std::nullopt;
+        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;
 }
 
 // Get account_info

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

@@ -21,7 +21,7 @@
 #include <xrpld/ledger/View.h>
 #include <xrpl/protocol/STObject.h>
 #include <xrpl/protocol/TxFlags.h>
-
+#include <map>
 #include <optional>
 
 namespace ripple {
@@ -32,7 +32,7 @@
     if (!ctx.rules.enabled(featurePermissionedDomains))
         return temDISABLED;
     if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
         return ret;
 
     auto const credentials = ctx.tx.getFieldArray(sfAcceptedCredentials);
     if (credentials.empty() || credentials.size() > PD_ARRAY_MAX)
@@ -54,7 +54,7 @@
 PermissionedDomainSet::preclaim(PreclaimContext const& ctx)
 {
     if (!ctx.view.read(keylet::account(ctx.tx.getAccountID(sfAccount))))
         return tefINTERNAL;
 
     assert(ctx.tx.isFieldPresent(sfAcceptedCredentials));
     auto const credentials = ctx.tx.getFieldArray(sfAcceptedCredentials);
@@ -95,12 +95,19 @@
 
     // All checks have already been done. Just update credentials. Same logic
     // 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);
-        // TODO when credentials are merged, it is possible that this will
-        // also need to sort on the CredentialType field in case there
-        // are multiple issuers in each set of credentials. The spec
-        // needs to be ironed out.
+        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);
+        }
+
         credentials.sort(
             [](STObject const& left, STObject const& right) -> bool {
                 if (left.getAccountID(sfIssuer) < right.getAccountID(sfIssuer))
@@ -112,11 +119,6 @@
                     {
                         return true;
                     }
-                    if (left.getFieldVL(sfCredentialType) ==
-                        right.getFieldVL(sfCredentialType))
-                    {
-                        throw std::runtime_error("duplicate credentials");
-                    }
                 }
                 return false;
             });
@@ -128,14 +130,7 @@
         // Modify existing permissioned domain.
         auto sleUpdate = view().peek(
             {ltPERMISSIONED_DOMAIN, ctx_.tx.getFieldH256(sfDomainID)});
-        try
-        {
-            updateSle(sleUpdate);
-        }
-        catch (...)
-        {
-            return temMALFORMED;
-        }
+        updateSle(sleUpdate);
         view().update(sleUpdate);
     }
     else
@@ -146,19 +141,12 @@
         auto slePd = std::make_shared<SLE>(pdKeylet);
         slePd->setAccountID(sfOwner, account_);
         slePd->setFieldU32(sfSequence, ctx_.tx.getFieldU32(sfSequence));
-        try
-        {
-            updateSle(slePd);
-        }
-        catch (...)
-        {
-            return temMALFORMED;
-        }
+        updateSle(slePd);
         view().insert(slePd);
         auto const page = view().dirInsert(
             keylet::ownerDir(account_), pdKeylet, describeOwnerDir(account_));
         if (!page)
             return tecDIR_FULL;
         slePd->setFieldU64(sfOwnerNode, *page);
         // If we succeeded, the new entry counts against the creator's reserve.
         adjustOwnerCount(view(), ownerSle, 1, ctx_.journal);