Merge fixAMMClawbackRounding amendment into featureAMMClawback amendment

Due to rounding, the LPTokenBalance of the last LP might not match the LP's trustline balance. This was fixed for `AMMWithdraw` in `fixAMMv1_1` by adjusting the LPTokenBalance to be the same as the trustline balance. Since `AMMClawback` is also performing a withdrawal, we need to adjust LPTokenBalance as well in `AMMClawback.`

This change includes:
1. Refactored `verifyAndAdjustLPTokenBalance` function in `AMMUtils`, which both`AMMWithdraw` and `AMMClawback` call to adjust LPTokenBalance.
2. Added the unit test `testLastHolderLPTokenBalance` to test the scenario.
3. Modify the existing unit tests for `fixAMMClawbackRounding`.

include/xrpl/protocol/Feature.h

@@ -80,7 +80,7 @@ namespace detail {
 // Feature.cpp. Because it's only used to reserve storage, and determine how
 // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
 // the actual number of amendments. A LogicError on startup will verify this.
-static constexpr std::size_t numFeatures = 114;
+static constexpr std::size_t numFeatures = 113;
 
 /** Amendments that this server supports and the default voting behavior.
    Whether they are enabled depends on the Rules defined in the validated

include/xrpl/protocol/detail/features.macro

@@ -31,7 +31,6 @@
 // If you add an amendment here, then do not forget to increment `numFeatures`
 // in include/xrpl/protocol/Feature.h.
 
-XRPL_FIX    (ImportIssuer,               Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FEATURE(HookAPISerializedType240,   Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(PermissionedDomains,        Supported::no,  VoteBehavior::DefaultNo)
 XRPL_FEATURE(DynamicNFT,                 Supported::no,  VoteBehavior::DefaultNo)

src/test/app/ClaimReward_test.cpp

@@ -18,7 +18,6 @@
 //==============================================================================
 
 #include <test/jtx.h>
-#include <test/jtx/AMM.h>
 #include <xrpld/app/ledger/LedgerMaster.h>
 #include <xrpl/protocol/Feature.h>
 #include <xrpl/protocol/jss.h>
@@ -301,28 +300,6 @@ struct ClaimReward_test : public beast::unit_test::suite
             env(tx, reward::issuer(issuer), ter(tecNO_ISSUER));
             env.close();
         }
-
-        // tecNO_PERMISSION
-        // issuer is an AMM account
-        {
-            test::jtx::Env env{*this, network::makeNetworkConfig(21337)};
-
-            auto const alice = Account("alice");
-            auto const issuer = Account("issuer");
-            auto const USD = issuer["USD"];
-
-            env.fund(XRP(1000), alice, issuer);
-            env.close();
-
-            AMM amm(env, issuer, XRP(100), USD(100));
-
-            BEAST_EXPECT(amm.ammExists());
-
-            env(reward::claim(alice),
-                reward::issuer(amm.ammAccount()),
-                ter(tecNO_PERMISSION));
-            env.close();
-        }
     }
 
     void

src/test/app/Import_test.cpp

@@ -19,7 +19,6 @@
 
 #include <test/app/Import_json.h>
 #include <test/jtx.h>
-#include <test/jtx/AMM.h>
 #include <xrpld/app/ledger/LedgerMaster.h>
 #include <xrpld/app/misc/AmendmentTable.h>
 #include <xrpld/app/misc/HashRouter.h>
@@ -2636,77 +2635,6 @@ class Import_test : public beast::unit_test::suite
                 ter(temDISABLED));
         }
 
-        // tecNO_ISSUER, tecNO_PERMISSION
-        // issuer not found, issuer is an AMM account
-        {
-            auto const alice = Account("alice");
-            auto const bob = Account("bob");
-            auto const issuer = Account("issuer");
-            auto const USD = issuer["USD"];
-
-            for (bool const withFixImportIssuer : {true, false})
-            {
-                auto const amend =
-                    withFixImportIssuer ? features : features - fixImportIssuer;
-                test::jtx::Env env{
-                    *this, network::makeNetworkVLConfig(21337, keys), amend};
-                env.fund(XRP(1000), alice, issuer);
-                env.close();
-
-                // burn 10'000 xrp
-                auto const master = Account("masterpassphrase");
-                env(noop(master), fee(10'000'000'000), ter(tesSUCCESS));
-                env.close();
-
-                env(import::import(
-                        alice, import::loadXpop(ImportTCAccountSet::w_seed)),
-                    import::issuer(bob),
-                    fee(100'000'000),
-                    withFixImportIssuer ? ter(tecNO_ISSUER) : ter(tesSUCCESS));
-                env.close();
-            }
-            for (bool const withFixImportIssuer : {true, false})
-            {
-                auto const amend =
-                    withFixImportIssuer ? features : features - fixImportIssuer;
-                test::jtx::Env env{
-                    *this, network::makeNetworkVLConfig(21337, keys), amend};
-                env.fund(XRP(1000), alice, issuer);
-                env.close();
-
-                // burn 10'000 xrp
-                auto const master = Account("masterpassphrase");
-                env(noop(master), fee(10'000'000'000), ter(tesSUCCESS));
-                env.close();
-
-                AMM amm(env, issuer, XRP(100), USD(100));
-                BEAST_EXPECT(amm.ammExists());
-
-                env(import::import(
-                        alice, import::loadXpop(ImportTCAccountSet::w_seed)),
-                    import::issuer(amm.ammAccount()),
-                    fee(100'000'000),
-                    withFixImportIssuer ? ter(tecNO_PERMISSION)
-                                        : ter(tesSUCCESS));
-                env.close();
-            }
-
-            //     env.enableFeature(fixImportIssuer);
-            //     env.close();
-
-            //     env(import::import(
-            //             carol, import::loadXpop(ImportTCAccountSet::w_seed)),
-            //         import::issuer(bob),
-            //         ter(tecNO_ISSUER));
-            //     env.close();
-            //     env(import::import(
-            //             dave, import::loadXpop(ImportTCAccountSet::w_seed)),
-            //         import::issuer(amm.ammAccount()),
-            //         // fee(100'000'000),
-            //         ter(tecNO_PERMISSION));
-            //     env.close();
-        }
-
         // tefINTERNAL
         // during preclaim could not parse xpop, bailing.
         {

src/test/app/Invoke_test.cpp

@@ -18,7 +18,6 @@
 //==============================================================================
 
 #include <test/jtx.h>
-#include <test/jtx/AMM.h>
 #include <xrpl/protocol/jss.h>
 
 namespace ripple {
@@ -169,27 +168,6 @@ class Invoke_test : public beast::unit_test::suite
                 fee(feeDrops),
                 ter(tecNO_TARGET));
         }
-
-        // tecNO_PERMISSION
-        // issuer is an AMM account
-        {
-            test::jtx::Env env{*this, network::makeNetworkConfig(21337)};
-
-            auto const alice = Account("alice");
-            auto const issuer = Account("issuer");
-            auto const USD = issuer["USD"];
-            env.fund(XRP(1000), alice, issuer);
-            env.close();
-
-            AMM amm(env, issuer, XRP(100), USD(100));
-
-            BEAST_EXPECT(amm.ammExists());
-
-            env(invoke::invoke(alice),
-                invoke::dest(amm.ammAccount()),
-                ter(tecNO_PERMISSION));
-            env.close();
-        }
     }
 
     void

src/test/app/Remit_test.cpp

@@ -18,7 +18,6 @@
 //==============================================================================
 
 #include <test/jtx.h>
-#include <test/jtx/AMM.h>
 #include <xrpld/core/ConfigSections.h>
 #include <xrpld/ledger/Dir.h>
 #include <xrpl/basics/chrono.h>
@@ -411,21 +410,6 @@ struct Remit_test : public beast::unit_test::suite
             env.close();
         }
 
-        // tecNO_PERMISSION - inform account is an AMM
-        {
-            Env env{*this, features};
-            env.fund(XRP(1000), gw, alice, bob);
-            env.close();
-
-            AMM amm(env, gw, XRP(100), USD(100));
-            BEAST_EXPECT(amm.ammExists());
-
-            auto tx = remit::remit(alice, bob);
-            tx[sfInform.jsonName] = to_string(amm.ammAccount());
-            env(tx, alice, ter(tecNO_PERMISSION));
-            env.close();
-        }
-
         // tecNO_PERMISSION - lsfDisallowIncomingRemit
         // DA: see testAllowIncoming
 

src/test/jtx/impl/import.cpp

@@ -44,7 +44,7 @@ import(jtx::Account const& account, Json::Value const& xpop)
 void
 issuer::operator()(Env& env, JTx& jt) const
 {
-    jt.jv[sfIssuer.jsonName] = to_string(issuer_);
+    jt.jv[sfIssuer.jsonName] = issuer_.human();
 }
 
 Json::Value

src/test/jtx/impl/invoke.cpp

@@ -66,7 +66,7 @@ blob::operator()(Env& env, JTx& jt) const
 void
 dest::operator()(Env& env, JTx& jt) const
 {
-    jt.jv[sfDestination.jsonName] = to_string(dest_);
+    jt.jv[sfDestination.jsonName] = dest_.human();
 }
 
 }  // namespace invoke

src/test/jtx/impl/reward.cpp

@@ -29,24 +29,18 @@ namespace reward {
 // Claim a reward.
 Json::Value
 claim(jtx::Account const& account)
-{
-    return claim(account.id());
-}
-
-Json::Value
-claim(AccountID const& account)
 {
     using namespace jtx;
     Json::Value jv;
     jv[jss::TransactionType] = jss::ClaimReward;
-    jv[jss::Account] = to_string(account);
+    jv[jss::Account] = account.human();
     return jv;
 }
 
 void
 issuer::operator()(Env& env, JTx& jt) const
 {
-    jt.jv[sfIssuer.jsonName] = to_string(issuer_);
+    jt.jv[sfIssuer.jsonName] = issuer_.human();
 }
 
 }  // namespace reward

src/test/jtx/import.h

@@ -37,10 +37,10 @@ import(jtx::Account const& account, Json::Value const& xpop);
 class issuer
 {
 private:
-    AccountID issuer_;
+    jtx::Account issuer_;
 
 public:
-    explicit issuer(AccountID const& issuer) : issuer_(issuer)
+    explicit issuer(jtx::Account const& issuer) : issuer_(issuer)
     {
     }
 

src/test/jtx/invoke.h

@@ -58,10 +58,10 @@ public:
 class dest
 {
 private:
-    AccountID dest_;
+    jtx::Account dest_;
 
 public:
-    explicit dest(AccountID const& dest) : dest_(dest)
+    explicit dest(jtx::Account const& dest) : dest_(dest)
     {
     }
 

src/test/jtx/reward.h

@@ -34,24 +34,17 @@ namespace reward {
 Json::Value
 claim(jtx::Account const& account);
 
-Json::Value
-claim(AccountID const& account);
-
 /** Sets the optional Issuer on a JTx. */
 class issuer
 {
 private:
-    AccountID issuer_;
+    jtx::Account issuer_;
 
 public:
     explicit issuer(jtx::Account const& issuer) : issuer_(issuer)
     {
     }
 
-    explicit issuer(AccountID const& issuer) : issuer_(issuer)
-    {
-    }
-
     void
     operator()(Env&, JTx& jtx) const;
 };

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

@@ -82,15 +82,8 @@ ClaimReward::preclaim(PreclaimContext const& ctx)
     if ((issuer && isOptOut) || (!issuer && !isOptOut))
         return temMALFORMED;
 
-    if (issuer)
-    {
-        auto const sleIssuer = ctx.view.read(keylet::account(*issuer));
-        if (!sleIssuer)
-            return tecNO_ISSUER;
-
-        if (sleIssuer->isFieldPresent(sfAMMID))
-            return tecNO_PERMISSION;
-    }
+    if (issuer && !ctx.view.exists(keylet::account(*issuer)))
+        return tecNO_ISSUER;
 
     return tesSUCCESS;
 }

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

@@ -870,17 +870,6 @@ Import::preclaim(PreclaimContext const& ctx)
     if (!ctx.tx.isFieldPresent(sfBlob))
         return tefINTERNAL;
 
-    if (ctx.tx.isFieldPresent(sfIssuer) &&
-        ctx.view.rules().enabled(fixImportIssuer))
-    {
-        auto const sleIssuer = ctx.view.read(keylet::account(ctx.tx[sfIssuer]));
-        if (!sleIssuer)
-            return tecNO_ISSUER;
-
-        if (sleIssuer->isFieldPresent(sfAMMID))
-            return tecNO_PERMISSION;
-    }
-
     // parse blob as json
     auto const xpop = syntaxCheckXPOP(ctx.tx.getFieldVL(sfBlob), ctx.j);
 

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

@@ -64,13 +64,8 @@ Invoke::preclaim(PreclaimContext const& ctx)
 
     if (ctx.tx.isFieldPresent(sfDestination))
     {
-        auto const sleDest =
-            ctx.view.read(keylet::account(ctx.tx[sfDestination]));
-        if (!sleDest)
+        if (!ctx.view.exists(keylet::account(ctx.tx[sfDestination])))
             return tecNO_TARGET;
-
-        if (sleDest->isFieldPresent(sfAMMID))
-            return tecNO_PERMISSION;
     }
 
     return tesSUCCESS;

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

@@ -256,18 +256,11 @@ Remit::doApply()
     if (ctx_.tx.isFieldPresent(sfInform))
     {
         auto const informAcc = ctx_.tx.getAccountID(sfInform);
-        auto const sleInformAcc = sb.read(keylet::account(informAcc));
-        if (!sleInformAcc)
+        if (!sb.exists(keylet::account(informAcc)))
         {
             JLOG(j.warn()) << "Remit: sfInform account does not exist.";
             return tecNO_TARGET;
         }
-
-        if (sleInformAcc->isFieldPresent(sfAMMID))
-        {
-            JLOG(j.warn()) << "Remit: sfInform account is an AMM.";
-            return tecNO_PERMISSION;
-        }
     }
 
     XRPAmount const accountReserve{sb.fees().accountReserve(0)};