remove unnecessary check

XRPLF · Sep 19, 2024 · ad78c7c · ad78c7c
1 parent 7a4429e
commit ad78c7c
Show file tree

Hide file tree

Showing 5 changed files with 91 additions and 20 deletions.
diff --git a/src/test/app/AMMClawback_test.cpp b/src/test/app/AMMClawback_test.cpp
@@ -293,6 +293,78 @@ class AMMClawback_test : public jtx::AMMTest
                     tfClawTwoAssets),
                 ter(tecNO_PERMISSION));
         }
+
+        // test clawing back xrp will return error
+        {
+            Env env(*this, features);
+            Account gw{"gateway"};
+            Account alice{"alice"};
+            env.fund(XRP(1000000), gw, alice);
+            env.close();
+
+            // gateway sets asfAllowTrustLineClawback
+            env(fset(gw, asfAllowTrustLineClawback));
+            env.close();
+            env.require(flags(gw, asfAllowTrustLineClawback));
+
+            // gateway issues 3000 USD to Alice
+            auto const USD = gw["USD"];
+            env.trust(USD(100000), alice);
+            env(pay(gw, alice, USD(3000)));
+            env.close();
+
+            // Alice creates AMM pool of XRP/USD
+            AMM amm(env, alice, XRP(1000), USD(2000), ter(tesSUCCESS));
+            env.close();
+
+            // clawback XRP is prohibited.
+            env(ammClawback(
+                    gw,
+                    alice,
+                    XRP,
+                    std::nullopt,
+                    amm.ammAccount(),
+                    std::nullopt),
+                ter(temMALFORMED));
+        }
+    }
+
+    void
+    testFeatureDisabled(FeatureBitset features)
+    {
+        testcase("test featureAMMClawback is not enabled.");
+        using namespace jtx;
+        if (!features[featureAMMClawback])
+        {
+            Env env(*this, features);
+            Account gw{"gateway"};
+            Account alice{"alice"};
+            env.fund(XRP(1000000), gw, alice);
+            env.close();
+
+            // gateway sets asfAllowTrustLineClawback
+            env(fset(gw, asfAllowTrustLineClawback));
+            env.close();
+            env.require(flags(gw, asfAllowTrustLineClawback));
+
+            // gateway issues 3000 USD to Alice
+            auto const USD = gw["USD"];
+            env.trust(USD(100000), alice);
+            env(pay(gw, alice, USD(3000)));
+            env.close();
+
+            // When featureAMMClawback is not enabled, AMMClawback is disabled.
+            // Because when featureAMMClawback is disabled, we can not create amm account,
+            // use gw account for now for testing purpose.
+            env(ammClawback(
+                    gw,
+                    alice,
+                    XRP,
+                    std::nullopt,
+                    gw.id(),
+                    std::nullopt),
+                ter(temDISABLED));
+        }
     }
 
     void
@@ -1469,7 +1541,9 @@ class AMMClawback_test : public jtx::AMMTest
     void
     testNotHoldingLptoken(FeatureBitset features)
     {
-        testcase("test AMMClawback from account which does not own any lptoken in the pool");
+        testcase(
+            "test AMMClawback from account which does not own any lptoken in "
+            "the pool");
         using namespace jtx;
 
         Env env(*this, features);
@@ -1503,6 +1577,7 @@ class AMMClawback_test : public jtx::AMMTest
     {
         FeatureBitset const all{jtx::supported_amendments()};
         testInvalidRequest(all);
+        testFeatureDisabled(all - featureAMMClawback);
         testAMMClawbackSpecificAmount(all);
         testAMMClawbackExceedBalance(all);
         testAMMClawbackAll(all);

diff --git a/src/test/jtx/impl/AMM.cpp b/src/test/jtx/impl/AMM.cpp
@@ -812,7 +812,6 @@ trust(AccountID const& account, STAmount const& amount, std::uint32_t flags)
     jv[jss::Flags] = flags;
     return jv;
 }
-
 Json::Value
 pay(Account const& account, AccountID const& to, STAmount const& amount)
 {

diff --git a/src/xrpld/app/tx/detail/AMMClawback.cpp b/src/xrpld/app/tx/detail/AMMClawback.cpp
@@ -170,27 +170,20 @@ AMMClawback::applyGuts(Sandbox& sb)
     Issue const asset = ctx_.tx[sfAsset];
 
     auto const sleAMMAccount = ctx_.view().read(keylet::account(ammAccount));
+
+    // should not happen. checked in preclaim.
+    // LCOV_EXCL_START
     if (!sleAMMAccount)
-    {
-        JLOG(j_.trace()) << "AMMClawback: AMMAccount provided does not exist.";
         return {terNO_AMM, false};
-    }
 
     auto const ammID = sleAMMAccount->getFieldH256(sfAMMID);
     if (!ammID)
-    {
-        JLOG(j_.trace())
-            << "AMMClawback: AMMAccount field is not an AMM account.";
         return {tecINTERNAL, false};
-    }
 
     auto ammSle = sb.peek(keylet::amm(ammID));
     if (!ammSle)
-    {
-        JLOG(j_.trace()) << "AMMClawback: can not find AMM with ammID: "
-                         << ammID;
         return {tecINTERNAL, false};
-    }
+    // LCOV_EXCL_STOP
 
     auto const tfee = getTradingFee(ctx_.view(), *ammSle, ammAccount);
     Issue const& issue1 = ammSle->getFieldIssue(sfAsset).issue();

diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.cpp b/src/xrpld/app/tx/detail/AMMWithdraw.cpp
@@ -399,7 +399,8 @@ AMMWithdraw::applyGuts(Sandbox& sb)
         {
             TER result;
             STAmount newLPTokenBalance;
-            bool withdrawAll = ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll);
+            bool withdrawAll =
+                ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll);
             std::tie(result, newLPTokenBalance, std::ignore, std::ignore) =
                 equalWithdrawTokens(
                     sb,
@@ -570,7 +571,8 @@ AMMWithdraw::equalWithdrawLimit(
     STAmount newLPTokenBalance;
     auto frac = Number{amount} / amountBalance;
     auto const amount2Withdraw = amount2Balance * frac;
-    bool withdrawAll = ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll);
+    bool withdrawAll =
+        ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll);
     if (amount2Withdraw <= amount2)
     {
         std::tie(result, newLPTokenBalance, std::ignore, std::ignore) =
@@ -632,7 +634,8 @@ AMMWithdraw::singleWithdraw(
 
     TER result;
     STAmount newLPTokenBalance;
-    bool withdrawAll = ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll);
+    bool withdrawAll =
+        ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll);
     std::tie(result, newLPTokenBalance, std::ignore, std::ignore) = withdraw(
         view,
         ammAccount,
@@ -678,7 +681,8 @@ AMMWithdraw::singleWithdrawTokens(
     {
         TER result;
         STAmount newLPTokenBalance;
-        bool withdrawAll = ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll);
+        bool withdrawAll =
+            ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll);
         std::tie(result, newLPTokenBalance, std::ignore, std::ignore) =
             withdraw(
                 view,
@@ -752,7 +756,8 @@ AMMWithdraw::singleWithdrawEPrice(
     {
         TER result;
         STAmount newLPTokenBalance;
-        bool withdrawAll = ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll);
+        bool withdrawAll =
+            ctx_.tx[sfFlags] & (tfWithdrawAll | tfOneAssetWithdrawAll);
         std::tie(result, newLPTokenBalance, std::ignore, std::ignore) =
             withdraw(
                 view,

diff --git a/src/xrpld/app/tx/detail/InvariantCheck.h b/src/xrpld/app/tx/detail/InvariantCheck.h
@@ -465,8 +465,7 @@ using InvariantChecks = std::tuple<
     ValidNewAccountRoot,
     ValidNFTokenPage,
     NFTokenCountTracking,
-    ValidClawback,
-    ValidAMMClawback>;
+    ValidClawback>;
 
 /**
  * @brief get a tuple of all invariant checks