removed amendmend code

Signed-off-by: Pratik Mankawde <pmankawde@ripple.com>

include/xrpl/protocol/TxFlags.h

@@ -211,18 +211,13 @@ constexpr std::uint32_t const tfMPTokenIssuanceDestroyMask  = ~tfUniversal;
 // issuer's reserve without bound.
 //
 // The fixRemoveNFTokenAutoTrustLine amendment disables minting with the
-// tfTrustLine flag as a way to prevent the attack.  But until the
+// tfTrustLine flag as a way to prevent the attack. Since the amendment
-// amendment passes we still need to keep the old behavior available.
+// has passed, we don't include tfTrustLine flag in tfNFTokenMintMask anymore.
+
 constexpr std::uint32_t const tfNFTokenMintMask =
     ~(tfUniversal | tfBurnable | tfOnlyXRP | tfTransferable);
 
-constexpr std::uint32_t const tfNFTokenMintOldMask =
+// if featureDynamicNFT enabled then flag allowing mutable URI available.
-    ~( ~tfNFTokenMintMask | tfTrustLine);
-
-// if featureDynamicNFT enabled then new flag allowing mutable URI available.
-constexpr std::uint32_t const tfNFTokenMintOldMaskWithMutable =
-    ~( ~tfNFTokenMintOldMask | tfMutable);
-
 constexpr std::uint32_t const tfNFTokenMintMaskWithMutable =
     ~( ~tfNFTokenMintMask | tfMutable);
 

include/xrpl/protocol/detail/features.macro

@@ -86,7 +86,6 @@ XRPL_FIX    (UniversalNumber,            Supported::yes, VoteBehavior::DefaultNo
 XRPL_FEATURE(XRPFees,                    Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(DisallowIncoming,           Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(ImmediateOfferKilled,       Supported::yes, VoteBehavior::DefaultNo)
-XRPL_FIX    (RemoveNFTokenAutoTrustLine, Supported::yes, VoteBehavior::DefaultYes)
 XRPL_FIX    (TrustLinesToSelf,           Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(NonFungibleTokensV1_1,      Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(ExpandedSignerList,         Supported::yes, VoteBehavior::DefaultNo)
@@ -155,3 +154,4 @@ XRPL_RETIRE(FlowCross)
 XRPL_RETIRE(fix1513)
 XRPL_RETIRE(fix1515)
 XRPL_RETIRE(fix1543)
+XRPL_RETIRE(fixRemoveNFTokenAutoTrustLine)

src/test/app/NFToken_test.cpp

@@ -1621,180 +1621,78 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
         IOU const gwCAD(gw["CAD"]);
         IOU const gwEUR(gw["EUR"]);
 
-        // The behavior of this test changes dramatically based on the
+        Env env{*this, features};
-        // presence (or absence) of the fixRemoveNFTokenAutoTrustLine
+        env.fund(XRP(1000), alice, becky, cheri, gw);
-        // amendment.  So we test both cases here.
+        env.close();
-        for (auto const& tweakedFeatures :
+
-             {features - fixRemoveNFTokenAutoTrustLine,
+        // Set trust lines so becky and cheri can use gw's currency.
-              features | fixRemoveNFTokenAutoTrustLine})
+        env(trust(becky, gwAUD(1000)));
+        env(trust(cheri, gwAUD(1000)));
+        env(trust(becky, gwCAD(1000)));
+        env(trust(cheri, gwCAD(1000)));
+        env(trust(becky, gwEUR(1000)));
+        env(trust(cheri, gwEUR(1000)));
+        env.close();
+        env(pay(gw, becky, gwAUD(500)));
+        env(pay(gw, becky, gwCAD(500)));
+        env(pay(gw, becky, gwEUR(500)));
+        env(pay(gw, cheri, gwAUD(500)));
+        env(pay(gw, cheri, gwCAD(500)));
+        env.close();
+
+        // An nft without flagCreateTrustLines but with a non-zero transfer
+        // fee will not allow creating offers that use IOUs for payment.
+        for (std::uint32_t xferFee : {0, 1})
         {
-            Env env{*this, tweakedFeatures};
+            uint256 const nftNoAutoTrustID{
-            env.fund(XRP(1000), alice, becky, cheri, gw);
+                token::getNextID(env, alice, 0u, tfTransferable, xferFee)};
+            env(token::mint(alice, 0u),
+                token::xferFee(xferFee),
+                txflags(tfTransferable));
             env.close();
 
-            // Set trust lines so becky and cheri can use gw's currency.
+            // becky buys the nft for 1 drop.
-            env(trust(becky, gwAUD(1000)));
+            uint256 const beckyBuyOfferIndex =
-            env(trust(cheri, gwAUD(1000)));
+                keylet::nftoffer(becky, env.seq(becky)).key;
-            env(trust(becky, gwCAD(1000)));
+            env(token::createOffer(becky, nftNoAutoTrustID, drops(1)),
-            env(trust(cheri, gwCAD(1000)));
+                token::owner(alice));
-            env(trust(becky, gwEUR(1000)));
-            env(trust(cheri, gwEUR(1000)));
             env.close();
-            env(pay(gw, becky, gwAUD(500)));
+            env(token::acceptBuyOffer(alice, beckyBuyOfferIndex));
-            env(pay(gw, becky, gwCAD(500)));
-            env(pay(gw, becky, gwEUR(500)));
-            env(pay(gw, cheri, gwAUD(500)));
-            env(pay(gw, cheri, gwCAD(500)));
             env.close();
 
-            // An nft without flagCreateTrustLines but with a non-zero transfer
+            // becky attempts to sell the nft for AUD.
-            // fee will not allow creating offers that use IOUs for payment.
+            TER const createOfferTER =
-            for (std::uint32_t xferFee : {0, 1})
+                xferFee ? TER(tecNO_LINE) : TER(tesSUCCESS);
-            {
+            uint256 const beckyOfferIndex =
-                uint256 const nftNoAutoTrustID{
+                keylet::nftoffer(becky, env.seq(becky)).key;
-                    token::getNextID(env, alice, 0u, tfTransferable, xferFee)};
+            env(token::createOffer(becky, nftNoAutoTrustID, gwAUD(100)),
-                env(token::mint(alice, 0u),
+                txflags(tfSellNFToken),
-                    token::xferFee(xferFee),
+                ter(createOfferTER));
-                    txflags(tfTransferable));
+            env.close();
-                env.close();
 
-                // becky buys the nft for 1 drop.
+            // cheri offers to buy the nft for CAD.
-                uint256 const beckyBuyOfferIndex =
+            uint256 const cheriOfferIndex =
-                    keylet::nftoffer(becky, env.seq(becky)).key;
+                keylet::nftoffer(cheri, env.seq(cheri)).key;
-                env(token::createOffer(becky, nftNoAutoTrustID, drops(1)),
+            env(token::createOffer(cheri, nftNoAutoTrustID, gwCAD(100)),
-                    token::owner(alice));
+                token::owner(becky),
-                env.close();
+                ter(createOfferTER));
-                env(token::acceptBuyOffer(alice, beckyBuyOfferIndex));
+            env.close();
-                env.close();
 
-                // becky attempts to sell the nft for AUD.
+            // To keep things tidy, cancel the offers.
-                TER const createOfferTER =
+            env(token::cancelOffer(becky, {beckyOfferIndex}));
-                    xferFee ? TER(tecNO_LINE) : TER(tesSUCCESS);
+            env(token::cancelOffer(cheri, {cheriOfferIndex}));
-                uint256 const beckyOfferIndex =
+            env.close();
-                    keylet::nftoffer(becky, env.seq(becky)).key;
-                env(token::createOffer(becky, nftNoAutoTrustID, gwAUD(100)),
-                    txflags(tfSellNFToken),
-                    ter(createOfferTER));
-                env.close();
-
-                // cheri offers to buy the nft for CAD.
-                uint256 const cheriOfferIndex =
-                    keylet::nftoffer(cheri, env.seq(cheri)).key;
-                env(token::createOffer(cheri, nftNoAutoTrustID, gwCAD(100)),
-                    token::owner(becky),
-                    ter(createOfferTER));
-                env.close();
-
-                // To keep things tidy, cancel the offers.
-                env(token::cancelOffer(becky, {beckyOfferIndex}));
-                env(token::cancelOffer(cheri, {cheriOfferIndex}));
-                env.close();
-            }
-            // An nft with flagCreateTrustLines but with a non-zero transfer
-            // fee allows transfers using IOUs for payment.
-            {
-                std::uint16_t transferFee = 10000;  // 10%
-
-                uint256 const nftAutoTrustID{token::getNextID(
-                    env, alice, 0u, tfTransferable | tfTrustLine, transferFee)};
-
-                // If the fixRemoveNFTokenAutoTrustLine amendment is active
-                // then this transaction fails.
-                {
-                    TER const mintTER =
-                        tweakedFeatures[fixRemoveNFTokenAutoTrustLine]
-                        ? static_cast<TER>(temINVALID_FLAG)
-                        : static_cast<TER>(tesSUCCESS);
-
-                    env(token::mint(alice, 0u),
-                        token::xferFee(transferFee),
-                        txflags(tfTransferable | tfTrustLine),
-                        ter(mintTER));
-                    env.close();
-
-                    // If fixRemoveNFTokenAutoTrustLine is active the rest
-                    // of this test falls on its face.
-                    if (tweakedFeatures[fixRemoveNFTokenAutoTrustLine])
-                        break;
-                }
-                // becky buys the nft for 1 drop.
-                uint256 const beckyBuyOfferIndex =
-                    keylet::nftoffer(becky, env.seq(becky)).key;
-                env(token::createOffer(becky, nftAutoTrustID, drops(1)),
-                    token::owner(alice));
-                env.close();
-                env(token::acceptBuyOffer(alice, beckyBuyOfferIndex));
-                env.close();
-
-                // becky sells the nft for AUD.
-                uint256 const beckySellOfferIndex =
-                    keylet::nftoffer(becky, env.seq(becky)).key;
-                env(token::createOffer(becky, nftAutoTrustID, gwAUD(100)),
-                    txflags(tfSellNFToken));
-                env.close();
-                env(token::acceptSellOffer(cheri, beckySellOfferIndex));
-                env.close();
-
-                // alice should now have a trust line for gwAUD.
-                BEAST_EXPECT(env.balance(alice, gwAUD) == gwAUD(10));
-
-                // becky buys the nft back for CAD.
-                uint256 const beckyBuyBackOfferIndex =
-                    keylet::nftoffer(becky, env.seq(becky)).key;
-                env(token::createOffer(becky, nftAutoTrustID, gwCAD(50)),
-                    token::owner(cheri));
-                env.close();
-                env(token::acceptBuyOffer(cheri, beckyBuyBackOfferIndex));
-                env.close();
-
-                // alice should now have a trust line for gwAUD and gwCAD.
-                BEAST_EXPECT(env.balance(alice, gwAUD) == gwAUD(10));
-                BEAST_EXPECT(env.balance(alice, gwCAD) == gwCAD(5));
-            }
-            // Now that alice has trust lines preestablished, an nft without
-            // flagCreateTrustLines will work for preestablished trust lines.
-            {
-                std::uint16_t transferFee = 5000;  // 5%
-                uint256 const nftNoAutoTrustID{token::getNextID(
-                    env, alice, 0u, tfTransferable, transferFee)};
-                env(token::mint(alice, 0u),
-                    token::xferFee(transferFee),
-                    txflags(tfTransferable));
-                env.close();
-
-                // alice sells the nft using AUD.
-                uint256 const aliceSellOfferIndex =
-                    keylet::nftoffer(alice, env.seq(alice)).key;
-                env(token::createOffer(alice, nftNoAutoTrustID, gwAUD(200)),
-                    txflags(tfSellNFToken));
-                env.close();
-                env(token::acceptSellOffer(cheri, aliceSellOfferIndex));
-                env.close();
-
-                // alice should now have AUD(210):
-                //  o 200 for this sale and
-                //  o 10 for the previous sale's fee.
-                BEAST_EXPECT(env.balance(alice, gwAUD) == gwAUD(210));
-
-                // cheri can't sell the NFT for EUR, but can for CAD.
-                env(token::createOffer(cheri, nftNoAutoTrustID, gwEUR(50)),
-                    txflags(tfSellNFToken),
-                    ter(tecNO_LINE));
-                env.close();
-                uint256 const cheriSellOfferIndex =
-                    keylet::nftoffer(cheri, env.seq(cheri)).key;
-                env(token::createOffer(cheri, nftNoAutoTrustID, gwCAD(100)),
-                    txflags(tfSellNFToken));
-                env.close();
-                env(token::acceptSellOffer(becky, cheriSellOfferIndex));
-                env.close();
-
-                // alice should now have CAD(10):
-                //  o 5 from this sale's fee and
-                //  o 5 for the previous sale's fee.
-                BEAST_EXPECT(env.balance(alice, gwCAD) == gwCAD(10));
-            }
         }
+        // An nft with flagCreateTrustLines but with a non-zero transfer
+        // fee allows transfers using IOUs for payment.
+
+        std::uint16_t transferFee = 10000;  // 10%
+
+        env(token::mint(alice, 0u),
+            token::xferFee(transferFee),
+            txflags(tfTransferable | tfTrustLine),
+            ter(static_cast<TER>(temINVALID_FLAG)));
+        env.close();
     }
 
     void
@@ -7433,12 +7331,12 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
         // https://github.com/XRPLF/rippled/issues/4925
         //
         // For an NFToken with a transfer fee, the issuer must be able to
-        // accept the transfer fee or else a transfer should fail.  If the
+        // accept the transfer fee or else a transfer should fail. If the
         // NFToken is transferred for a non-XRP asset, then the issuer must
         // have a trustline to that asset to receive the fee.
         //
         // This test looks at a situation where issuer would get a trustline
-        // for the fee without the issuer's consent.  Here are the steps:
+        // for the fee without the issuer's consent. Here are the steps:
         //  1. Issuer has a trustline (i.e., USD)
         //  2. Issuer mints NFToken with transfer fee.
         //  3. Becky acquires the NFToken, paying with XRP.
@@ -7456,8 +7354,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
         //
         // In both cases we remove the fixRemoveNFTokenAutoTrustLine amendment.
         // Otherwise we can't create NFTokens with tfTrustLine enabled.
-        FeatureBitset const localFeatures =
+        FeatureBitset const localFeatures = features;
-            features - fixRemoveNFTokenAutoTrustLine;
         for (FeatureBitset feats :
              {localFeatures - fixEnforceNFTokenTrustline,
               localFeatures | fixEnforceNFTokenTrustline})
@@ -7627,8 +7524,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
         //
         // We remove the fixRemoveNFTokenAutoTrustLine amendment.  Otherwise
         // we can't create NFTokens with tfTrustLine enabled.
-        FeatureBitset const localFeatures =
+        FeatureBitset const localFeatures = features;
-            features - fixRemoveNFTokenAutoTrustLine;
 
         Env env{*this, localFeatures};
         env.fund(XRP(1000), issuer, becky, cheri);

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

@@ -55,28 +55,11 @@ NFTokenMint::checkExtraFeatures(PreflightContext const& ctx)
 std::uint32_t
 NFTokenMint::getFlagsMask(PreflightContext const& ctx)
 {
-    // Prior to fixRemoveNFTokenAutoTrustLine, transfer of an NFToken between
-    // accounts allowed a TrustLine to be added to the issuer of that token
-    // without explicit permission from that issuer.  This was enabled by
-    // minting the NFToken with the tfTrustLine flag set.
-    //
-    // That capability could be used to attack the NFToken issuer.  It
-    // would be possible for two accounts to trade the NFToken back and forth
-    // building up any number of TrustLines on the issuer, increasing the
-    // issuer's reserve without bound.
-    //
-    // The fixRemoveNFTokenAutoTrustLine amendment disables minting with the
-    // tfTrustLine flag as a way to prevent the attack.  But until the
-    // amendment passes we still need to keep the old behavior available.
     std::uint32_t const nfTokenMintMask =
-        ctx.rules.enabled(fixRemoveNFTokenAutoTrustLine)
         // if featureDynamicNFT enabled then new flag allowing mutable URI
         // available
-        ? ctx.rules.enabled(featureDynamicNFT) ? tfNFTokenMintMaskWithMutable
+        ctx.rules.enabled(featureDynamicNFT) ? tfNFTokenMintMaskWithMutable
-                                               : tfNFTokenMintMask
+                                             : tfNFTokenMintMask;
-        : ctx.rules.enabled(featureDynamicNFT) ? tfNFTokenMintOldMaskWithMutable
-                                               : tfNFTokenMintOldMask;
-
     return nfTokenMintMask;
 }