fix verifyDepositPreauth

XRPLF · Nov 6, 2024 · 0e2683d · 0e2683d
1 parent ef82c3b
commit 0e2683d
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 62 deletions.
diff --git a/src/xrpld/app/misc/CredentialHelpers.cpp b/src/xrpld/app/misc/CredentialHelpers.cpp
@@ -212,12 +212,27 @@ authorized(ApplyContext const& ctx, AccountID const& dst)
     return tesSUCCESS;
 }
 
+std::set<std::pair<AccountID, Slice>>
+makeSorted(STArray const& in)
+{
+    std::set<std::pair<AccountID, Slice>> out;
+    for (auto const& cred : in)
+    {
+        auto [it, ins] = out.emplace(cred[sfIssuer], cred[sfCredentialType]);
+        if (!ins)
+            return {};
+    }
+    return out;
+}
+
+}  // namespace credentials
+
 TER
-verify(
+verifyDepositPreauth(
     ApplyContext& ctx,
     AccountID const& src,
     AccountID const& dst,
-    bool requireAuth)
+    std::shared_ptr<SLE> const& sleDst)
 {
     // If depositPreauth is enabled, then an account that requires
     // authorization has at least two ways to get a payment in:
@@ -230,43 +245,17 @@ verify(
         credentials::removeExpired(ctx.view(), ctx.tx, ctx.journal))
         return tecEXPIRED;
 
-    if (!requireAuth || (src == dst))
-        return tesSUCCESS;
-
-    if (ctx.view().exists(keylet::depositPreauth(dst, src)))
-        return tesSUCCESS;
-
-    if (!credentialsPresent)
-        return tecNO_PERMISSION;
-
-    return credentials::authorized(ctx, dst);
-}
-
-TER
-verify(
-    ApplyContext& ctx,
-    AccountID const& src,
-    AccountID const& dst,
-    std::optional<std::reference_wrapper<std::shared_ptr<SLE> const>> sleDstOpt)
-{
-    std::shared_ptr<SLE const> const& sleDst =
-        sleDstOpt ? *sleDstOpt : ctx.view().peek(keylet::account(dst));
-    return verify(
-        ctx, src, dst, sleDst && (sleDst->getFlags() & lsfDepositAuth));
-}
-
-std::set<std::pair<AccountID, Slice>>
-makeSorted(STArray const& in)
-{
-    std::set<std::pair<AccountID, Slice>> out;
-    for (auto const& cred : in)
+    if (sleDst && (sleDst->getFlags() & lsfDepositAuth))
     {
-        auto [it, ins] = out.emplace(cred[sfIssuer], cred[sfCredentialType]);
-        if (!ins)
-            return {};
+        if (src != dst)
+        {
+            if (!ctx.view().exists(keylet::depositPreauth(dst, src)))
+                return !credentialsPresent ? tecNO_PERMISSION
+                                           : credentials::authorized(ctx, dst);
+        }
     }
-    return out;
+
+    return tesSUCCESS;
 }
 
-}  // namespace credentials
 }  // namespace ripple
diff --git a/src/xrpld/app/misc/CredentialHelpers.h b/src/xrpld/app/misc/CredentialHelpers.h
@@ -60,25 +60,18 @@ valid(PreclaimContext const& ctx, AccountID const& src);
 TER
 authorized(ApplyContext const& ctx, AccountID const& dst);
 
-// Check expired credentials and for existing DepositPreauth ledger object
-TER
-verify(
-    ApplyContext& ctx,
-    AccountID const& src,
-    AccountID const& dst,
-    std::optional<std::reference_wrapper<std::shared_ptr<SLE> const>>
-        sleDstOpt = {});
+// return empty set if there are duplicates
+std::set<std::pair<AccountID, Slice>>
+makeSorted(STArray const& in);
 
+}  // namespace credentials
+
+// Check expired credentials and for existing DepositPreauth ledger object
 TER
-verify(
+verifyDepositPreauth(
     ApplyContext& ctx,
     AccountID const& src,
     AccountID const& dst,
-    bool requireAuth);
-
-// return empty set if there are duplicates
-std::set<std::pair<AccountID, Slice>>
-makeSorted(STArray const& in);
+    std::shared_ptr<SLE> const& sleDst);
 
-}  // namespace credentials
 }  // namespace ripple
diff --git a/src/xrpld/app/tx/detail/DeleteAccount.cpp b/src/xrpld/app/tx/detail/DeleteAccount.cpp
@@ -357,7 +357,7 @@ DeleteAccount::doApply()
     if (ctx_.view().rules().enabled(featureDepositAuth) &&
         ctx_.tx.isFieldPresent(sfCredentialIDs))
     {
-        if (auto err = credentials::verify(ctx_, account_, dstID, dst);
+        if (auto err = verifyDepositPreauth(ctx_, account_, dstID, dst);
             !isTesSuccess(err))
             return err;
     }

diff --git a/src/xrpld/app/tx/detail/Escrow.cpp b/src/xrpld/app/tx/detail/Escrow.cpp
@@ -477,7 +477,7 @@ EscrowFinish::doApply()
 
     if (ctx_.view().rules().enabled(featureDepositAuth))
     {
-        if (auto err = credentials::verify(ctx_, account_, destID, sled);
+        if (auto err = verifyDepositPreauth(ctx_, account_, destID, sled);
             !isTesSuccess(err))
             return err;
     }

diff --git a/src/xrpld/app/tx/detail/PayChan.cpp b/src/xrpld/app/tx/detail/PayChan.cpp
@@ -539,8 +539,7 @@ PayChanClaim::doApply()
 
         if (depositAuth)
         {
-            if (auto err = credentials::verify(
-                    ctx_, txAccount, dst, sled->getFlags() & lsfDepositAuth);
+            if (auto err = verifyDepositPreauth(ctx_, txAccount, dst, sled);
                 !isTesSuccess(err))
                 return err;
         }

diff --git a/src/xrpld/app/tx/detail/Payment.cpp b/src/xrpld/app/tx/detail/Payment.cpp
@@ -400,8 +400,8 @@ Payment::doApply()
             //  1. If Account == Destination, or
             //  2. If Account is deposit preauthorized by destination.
 
-            if (auto err = credentials::verify(
-                    ctx_, account_, dstAccountID, reqDepositAuth);
+            if (auto err =
+                    verifyDepositPreauth(ctx_, account_, dstAccountID, sleDst);
                 !isTesSuccess(err))
                 return err;
         }
@@ -472,8 +472,8 @@ Payment::doApply()
 
         if (view().rules().enabled(featureCredentials))
         {
-            if (auto err = credentials::verify(
-                    ctx_, account_, dstAccountID, reqDepositAuth);
+            if (auto err =
+                    verifyDepositPreauth(ctx_, account_, dstAccountID, sleDst);
                 !isTesSuccess(err))
                 return err;
         }
@@ -594,8 +594,8 @@ Payment::doApply()
         if (dstAmount > dstReserve ||
             sleDst->getFieldAmount(sfBalance) > dstReserve)
         {
-            if (auto err = credentials::verify(
-                    ctx_, account_, dstAccountID, reqDepositAuth);
+            if (auto err =
+                    verifyDepositPreauth(ctx_, account_, dstAccountID, sleDst);
                 !isTesSuccess(err))
                 return err;
         }