Align with XLS-70

include/xrpl/protocol/Protocol.h

@@ -104,6 +104,10 @@ std::size_t constexpr maxCredentialTypeLength = 64;
 /** The maximum number of credentials can be passed in array */
 std::size_t constexpr maxCredentialsArraySize = 8;
 
+/** The maximum number of credentials can be passed in array for permissioned
+ * domain */
+std::size_t constexpr maxPermissionedDomainCredentialsArraySize = 10;
+
 /** The maximum length of MPTokenMetadata */
 std::size_t constexpr maxMPTokenMetadataLength = 1024;
 

src/test/app/PermissionedDomains_test.cpp

@@ -128,7 +128,8 @@ class PermissionedDomains_test : public beast::unit_test::suite
             {alice11, "credential10"},
             {alice12, "credential11"}};
         BEAST_EXPECT(
-            credentials11.size() == PermissionedDomainSet::PD_ARRAY_MAX + 1);
+            credentials11.size() ==
+            maxPermissionedDomainCredentialsArraySize + 1);
         env(pd::setTx(account, credentials11, domain), ter(temMALFORMED));
 
         // Test credentials including non-existent issuer.
@@ -141,7 +142,7 @@ class PermissionedDomains_test : public beast::unit_test::suite
             {alice5, "credential5"},
             {alice6, "credential6"},
             {alice7, "credential7"}};
-        env(pd::setTx(account, credentialsNon, domain), ter(temBAD_ISSUER));
+        env(pd::setTx(account, credentialsNon, domain), ter(tecNO_ISSUER));
 
         pd::Credentials const credentials4{
             {alice2, "credential1"},
@@ -191,8 +192,12 @@ class PermissionedDomains_test : public beast::unit_test::suite
             for (auto const& c : credentialsDup)
                 pubKey2Acc.emplace(c.issuer.human(), c.issuer);
 
-            BEAST_EXPECT(pd::sortCredentials(credentialsDup).size() == 4);
-            env(pd::setTx(account, credentialsDup, domain));
+            auto const sorted = pd::sortCredentials(credentialsDup);
+            BEAST_EXPECT(sorted.size() == 4);
+            env(pd::setTx(account, credentialsDup, domain), ter(temMALFORMED));
+
+            env.close();
+            env(pd::setTx(account, sorted, domain));
 
             uint256 d;
             if (domain)
@@ -302,7 +307,8 @@ class PermissionedDomains_test : public beast::unit_test::suite
         uint256 domain2;
         {
             BEAST_EXPECT(
-                credentials10.size() == PermissionedDomainSet::PD_ARRAY_MAX);
+                credentials10.size() ==
+                maxPermissionedDomainCredentialsArraySize);
             BEAST_EXPECT(credentials10 != pd::sortCredentials(credentials10));
             env(pd::setTx(alice[0], credentials10));
             auto tx = env.tx()->getJson(JsonOptions::none);
@@ -330,8 +336,7 @@ class PermissionedDomains_test : public beast::unit_test::suite
             pd::sortCredentials(credentials10));
 
         // Update from the wrong owner.
-        env(pd::setTx(alice[2], credentials1, domain2),
-            ter(temINVALID_ACCOUNT_ID));
+        env(pd::setTx(alice[2], credentials1, domain2), ter(tecNO_PERMISSION));
 
         // Update a uint256(0) domain
         env(pd::setTx(alice[0], credentials1, uint256(0)), ter(temMALFORMED));
@@ -391,7 +396,7 @@ class PermissionedDomains_test : public beast::unit_test::suite
         // Delete a domain that doesn't belong to the account.
         Account const bob("bob");
         env.fund(XRP(1000), bob);
-        env(pd::deleteTx(bob, domain), ter(temINVALID_ACCOUNT_ID));
+        env(pd::deleteTx(bob, domain), ter(tecNO_PERMISSION));
 
         // Delete a non-existent domain.
         env(pd::deleteTx(alice, uint256(75)), ter(tecNO_ENTRY));

src/xrpld/app/misc/CredentialHelpers.cpp

@@ -19,6 +19,7 @@
 
 #include <xrpld/app/misc/CredentialHelpers.h>
 #include <xrpld/ledger/View.h>
+#include <xrpl/protocol/digest.h>
 
 #include <unordered_set>
 
@@ -225,6 +226,31 @@ makeSorted(STArray const& in)
     return out;
 }
 
+NotTEC
+checkArray(STArray const& in, unsigned maxSize)
+{
+    if (in.empty() || (in.size() > maxSize))
+        return temMALFORMED;
+
+    std::unordered_set<uint256> duplicates;
+    for (auto const& credential : in)
+    {
+        auto const& issuer(credential[sfIssuer]);
+        if (!issuer)
+            return temINVALID_ACCOUNT_ID;
+
+        auto const ct = credential[sfCredentialType];
+        if (ct.empty() || (ct.size() > maxCredentialTypeLength))
+            return temMALFORMED;
+
+        auto [it, ins] = duplicates.insert(sha512Half(issuer, ct));
+        if (!ins)
+            return temMALFORMED;
+    }
+
+    return tesSUCCESS;
+}
+
 }  // namespace credentials
 
 TER

src/xrpld/app/misc/CredentialHelpers.h

@@ -21,8 +21,6 @@
 
 #include <xrpld/app/tx/detail/Transactor.h>
 
-#include <optional>
-
 namespace ripple {
 namespace credentials {
 
@@ -60,10 +58,15 @@ valid(PreclaimContext const& ctx, AccountID const& src);
 TER
 authorized(ApplyContext const& ctx, AccountID const& dst);
 
-// return empty set if there are duplicates
+// Sort credentials array, return empty set if there are duplicates
 std::set<std::pair<AccountID, Slice>>
 makeSorted(STArray const& in);
 
+// Check credentials array passed to DepositPreauth/PermissionedDomainSet
+// transactions
+NotTEC
+checkArray(STArray const& in, unsigned maxSize);
+
 }  // namespace credentials
 
 // Check expired credentials and for existing DepositPreauth ledger object

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

@@ -24,11 +24,9 @@
 #include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/Indexes.h>
 #include <xrpl/protocol/TxFlags.h>
-#include <xrpl/protocol/digest.h>
 #include <xrpl/protocol/st.h>
 
 #include <optional>
-#include <unordered_set>
 
 namespace ripple {
 
@@ -94,45 +92,13 @@ DepositPreauth::preflight(PreflightContext const& ctx)
     }
     else
     {
-        STArray const& arr(ctx.tx.getFieldArray(
-            authArrPresent ? sfAuthorizeCredentials
-                           : sfUnauthorizeCredentials));
-        if (arr.empty() || (arr.size() > maxCredentialsArraySize))
-        {
-            JLOG(ctx.j.trace()) << "Malformed transaction: "
-                                   "Invalid AuthorizeCredentials size: "
-                                << arr.size();
-            return temMALFORMED;
-        }
-
-        std::unordered_set<uint256> duplicates;
-        for (auto const& o : arr)
-        {
-            auto const& issuer(o[sfIssuer]);
-            if (!issuer)
-            {
-                JLOG(ctx.j.trace())
-                    << "Malformed transaction: "
-                       "AuthorizeCredentials Issuer account is invalid.";
-                return temINVALID_ACCOUNT_ID;
-            }
-
-            auto const ct = o[sfCredentialType];
-            if (ct.empty() || (ct.size() > maxCredentialTypeLength))
-            {
-                JLOG(ctx.j.trace())
-                    << "Malformed transaction: invalid size of CredentialType.";
-                return temMALFORMED;
-            }
-
-            auto [it, ins] = duplicates.insert(sha512Half(issuer, ct));
-            if (!ins)
-            {
-                JLOG(ctx.j.trace())
-                    << "Malformed transaction: duplicates in credentials.";
-                return temMALFORMED;
-            }
-        }
+        if (auto err = credentials::checkArray(
+                ctx.tx.getFieldArray(
+                    authArrPresent ? sfAuthorizeCredentials
+                                   : sfUnauthorizeCredentials),
+                maxCredentialsArraySize);
+            !isTesSuccess(err))
+            return err;
     }
 
     return preflight2(ctx);

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

@@ -19,6 +19,7 @@
 
 #include <xrpld/app/tx/detail/InvariantCheck.h>
 
+#include <xrpld/app/misc/CredentialHelpers.h>
 #include <xrpld/app/tx/detail/NFTokenUtils.h>
 #include <xrpld/app/tx/detail/PermissionedDomainSet.h>
 #include <xrpld/ledger/ReadView.h>
@@ -1132,7 +1133,23 @@ ValidPermissionedDomain::visitEntry(
 {
     if (after->getType() != ltPERMISSIONED_DOMAIN)
         return;
-    credentialsSize_ = after->getFieldArray(sfAcceptedCredentials).size();
+    auto const& credentials = after->getFieldArray(sfAcceptedCredentials);
+    credentialsSize_ = credentials.size();
+    auto const sorted = credentials::makeSorted(credentials);
+    isUnique_ = !sorted.empty();
+
+    if (isUnique_)
+    {
+        unsigned i = 0;
+        for (auto const& cred : sorted)
+        {
+            auto const& credTx(credentials[i++]);
+            isSorted_ = (cred.first == credTx[sfIssuer]) &&
+                (cred.second == credTx[sfCredentialType]);
+            if (!isSorted_)
+                break;
+        }
+    }
 }
 
 bool
@@ -1153,14 +1170,28 @@ ValidPermissionedDomain::finalize(
         return false;
     }
 
-    if (credentialsSize_ > PermissionedDomainSet::PD_ARRAY_MAX)
+    if (credentialsSize_ > maxPermissionedDomainCredentialsArraySize)
     {
         JLOG(j.fatal()) << "Invariant failed: permissioned domain bad "
                            "credentials size "
                         << credentialsSize_;
         return false;
     }
 
+    if (!isSorted_)
+    {
+        JLOG(j.fatal()) << "Invariant failed: permissioned domain credentials "
+                           "aren't sorted";
+        return false;
+    }
+
+    if (!isUnique_)
+    {
+        JLOG(j.fatal()) << "Invariant failed: permissioned domain credentials "
+                           "aren't unique";
+        return false;
+    }
+
     return true;
 }
 

src/xrpld/app/tx/detail/InvariantCheck.h

@@ -486,6 +486,7 @@ class ValidMPTIssuance
 class ValidPermissionedDomain
 {
     std::size_t credentialsSize_{0};
+    bool isSorted_ = false, isUnique_ = false;
 
 public:
     void

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

@@ -29,23 +29,26 @@ PermissionedDomainDelete::preflight(PreflightContext const& ctx)
         return temDISABLED;
     if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
         return ret;
+
+    assert(ctx.tx.isFieldPresent(sfDomainID));
+    auto const domain = ctx.tx.getFieldH256(sfDomainID);
+    if (domain == beast::zero)
+        return temMALFORMED;
+
     return preflight2(ctx);
 }
 
 TER
 PermissionedDomainDelete::preclaim(PreclaimContext const& ctx)
 {
-    assert(ctx.tx.isFieldPresent(sfDomainID));
     auto const domain = ctx.tx.getFieldH256(sfDomainID);
-    if (domain == beast::zero)
-        return temMALFORMED;
     auto const sleDomain = ctx.view.read({ltPERMISSIONED_DOMAIN, domain});
     if (!sleDomain)
         return tecNO_ENTRY;
     assert(
         sleDomain->isFieldPresent(sfOwner) && ctx.tx.isFieldPresent(sfAccount));
     if (sleDomain->getAccountID(sfOwner) != ctx.tx.getAccountID(sfAccount))
-        return temINVALID_ACCOUNT_ID;
+        return tecNO_PERMISSION;
     return tesSUCCESS;
 }
 

src/xrpld/app/tx/detail/PermissionedDomainDelete.h

@@ -17,8 +17,7 @@
 */
 //==============================================================================
 
-#ifndef RIPPLE_TX_PERMISSIONEDDOMAINDELETE_H_INCLUDED
-#define RIPPLE_TX_PERMISSIONEDDOMAINDELETE_H_INCLUDED
+#pragma once
 
 #include <xrpld/app/tx/detail/Transactor.h>
 
@@ -39,11 +38,9 @@ class PermissionedDomainDelete : public Transactor
     static TER
     preclaim(PreclaimContext const& ctx);
 
-    /** Attempt to create the Permissioned Domain. */
+    /** Attempt to delete the Permissioned Domain. */
     TER
     doApply() override;
 };
 
 }  // namespace ripple
-
-#endif  // RIPPLE_TX_PERMISSIONEDDOMAINDELETE_H_INCLUDED