Disallow setting a AMM account as Issuer/Destination/Inform

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 = 113;
+static constexpr std::size_t numFeatures = 114;
 
 /** 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,6 +31,7 @@
 // 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,6 +18,7 @@
 //==============================================================================
 
 #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>
@@ -300,6 +301,28 @@ 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,6 +19,7 @@
 
 #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>
@@ -2635,6 +2636,77 @@ 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,6 +18,7 @@
 //==============================================================================
 
 #include <test/jtx.h>
+#include <test/jtx/AMM.h>
 #include <xrpl/protocol/jss.h>
 
 namespace ripple {
@@ -168,6 +169,27 @@ 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,6 +18,7 @@
 //==============================================================================
 
 #include <test/jtx.h>
+#include <test/jtx/AMM.h>
 #include <xrpld/core/ConfigSections.h>
 #include <xrpld/ledger/Dir.h>
 #include <xrpl/basics/chrono.h>
@@ -410,6 +411,21 @@ 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] = issuer_.human();
+    jt.jv[sfIssuer.jsonName] = to_string(issuer_);
 }
 
 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] = dest_.human();
+    jt.jv[sfDestination.jsonName] = to_string(dest_);
 }
 
 }  // namespace invoke

src/test/jtx/impl/reward.cpp

@@ -29,18 +29,24 @@ 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] = account.human();
+    jv[jss::Account] = to_string(account);
     return jv;
 }
 
 void
 issuer::operator()(Env& env, JTx& jt) const
 {
-    jt.jv[sfIssuer.jsonName] = issuer_.human();
+    jt.jv[sfIssuer.jsonName] = to_string(issuer_);
 }
 
 }  // namespace reward

src/test/jtx/import.h

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

src/test/jtx/invoke.h

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

src/test/jtx/reward.h

@@ -34,17 +34,24 @@ 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:
-    jtx::Account issuer_;
+    AccountID 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,8 +82,15 @@ ClaimReward::preclaim(PreclaimContext const& ctx)
     if ((issuer && isOptOut) || (!issuer && !isOptOut))
         return temMALFORMED;
 
-    if (issuer && !ctx.view.exists(keylet::account(*issuer)))
-        return tecNO_ISSUER;
+    if (issuer)
+    {
+        auto const sleIssuer = ctx.view.read(keylet::account(*issuer));
+        if (!sleIssuer)
+            return tecNO_ISSUER;
+
+        if (sleIssuer->isFieldPresent(sfAMMID))
+            return tecNO_PERMISSION;
+    }
 
     return tesSUCCESS;
 }

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

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

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

@@ -256,11 +256,18 @@ Remit::doApply()
     if (ctx_.tx.isFieldPresent(sfInform))
     {
         auto const informAcc = ctx_.tx.getAccountID(sfInform);
-        if (!sb.exists(keylet::account(informAcc)))
+        auto const sleInformAcc = sb.read(keylet::account(informAcc));
+        if (!sleInformAcc)
         {
             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)};