Add the fixEnforceNFTokenTrustline amendment: (XRPLF#4946)

Fix interactions between NFTokenOffers and trust lines.

Since the NFTokenAcceptOffer does not check the trust line that
the issuer receives as a transfer fee in the NFTokenAcceptOffer,
if the issuer deletes the trust line after NFTokenCreateOffer,
the trust line is created for the issuer by the
NFTokenAcceptOffer.  That's fixed.

Resolves XRPLF#4925.

src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp

@@ -270,6 +270,28 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
         }
     }
 
+    // Fix a bug where the transfer of an NFToken with a transfer fee could
+    // give the NFToken issuer an undesired trust line.
+    if (ctx.view.rules().enabled(fixEnforceNFTokenTrustline))
+    {
+        std::shared_ptr<SLE const> const& offer = bo ? bo : so;
+        if (!offer)
+            // Should be caught in preflight.
+            return tecINTERNAL;
+
+        uint256 const& tokenID = offer->at(sfNFTokenID);
+        STAmount const& amount = offer->at(sfAmount);
+        if (nft::getTransferFee(tokenID) != 0 &&
+            (nft::getFlags(tokenID) & nft::flagCreateTrustLines) == 0 &&
+            !amount.native())
+        {
+            auto const issuer = nft::getIssuer(tokenID);
+            // Issuer doesn't need a trust line to accept their own currency.
+            if (issuer != amount.getIssuer() &&
+                !ctx.view.read(keylet::line(issuer, amount.issue())))
+                return tecNO_LINE;
+        }
+    }
     return tesSUCCESS;
 }
 

src/ripple/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 = 75;
+static constexpr std::size_t numFeatures = 76;
 
 /** Amendments that this server supports and the default voting behavior.
    Whether they are enabled depends on the Rules defined in the validated
@@ -368,6 +368,7 @@ extern uint256 const fixPreviousTxnID;
 extern uint256 const fixAMMv1_1;
 extern uint256 const featureNFTokenMintOffer;
 extern uint256 const fixReducedOffersV2;
+extern uint256 const fixEnforceNFTokenTrustline;
 
 }  // namespace ripple
 

src/ripple/protocol/impl/Feature.cpp

@@ -495,6 +495,7 @@ REGISTER_FIX    (fixPreviousTxnID,              Supported::yes, VoteBehavior::De
 REGISTER_FIX    (fixAMMv1_1,                    Supported::yes, VoteBehavior::DefaultNo);
 REGISTER_FEATURE(NFTokenMintOffer,              Supported::yes, VoteBehavior::DefaultNo);
 REGISTER_FIX    (fixReducedOffersV2,            Supported::yes, VoteBehavior::DefaultNo);
+REGISTER_FIX    (fixEnforceNFTokenTrustline,    Supported::yes, VoteBehavior::DefaultNo);
 
 // The following amendments are obsolete, but must remain supported
 // because they could potentially get enabled.

src/test/app/NFToken_test.cpp

@@ -7416,6 +7416,176 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
         }
     }
 
+    void
+    testUnaskedForAutoTrustline(FeatureBitset features)
+    {
+        testcase("Test fix unasked for auto-trustline.");
+
+        using namespace test::jtx;
+
+        Account const issuer{"issuer"};
+        Account const becky{"becky"};
+        Account const cheri{"cheri"};
+        Account const gw("gw");
+        IOU const gwAUD(gw["AUD"]);
+
+        // This test case covers issue...
+        // 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
+        // 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:
+        //  1. Issuer has a trustline (i.e., USD)
+        //  2. Issuer mints NFToken with transfer fee.
+        //  3. Becky acquires the NFToken, paying with XRP.
+        //  4. Becky creates offer to sell NFToken for USD(100).
+        //  5. Issuer deletes trustline for USD.
+        //  6. Carol buys NFToken from Becky for USD(100).
+        //  7. The transfer fee from Carol's purchase re-establishes issuer's
+        //     USD trustline.
+        //
+        // The fixEnforceNFTokenTrustline amendment addresses this oversight.
+        //
+        // We run this test case both with and without
+        // fixEnforceNFTokenTrustline enabled so we can see the change
+        // in behavior.
+        //
+        // In both cases we remove the fixRemoveNFTokenAutoTrustLine amendment.
+        // Otherwise we can't create NFTokens with tfTrustLine enabled.
+        FeatureBitset const localFeatures =
+            features - fixRemoveNFTokenAutoTrustLine;
+        for (FeatureBitset feats :
+             {localFeatures - fixEnforceNFTokenTrustline,
+              localFeatures | fixEnforceNFTokenTrustline})
+        {
+            Env env{*this, feats};
+            env.fund(XRP(1000), issuer, becky, cheri, gw);
+            env.close();
+
+            // Set trust lines so becky and cheri can use gw's currency.
+            env(trust(becky, gwAUD(1000)));
+            env(trust(cheri, gwAUD(1000)));
+            env.close();
+            env(pay(gw, cheri, gwAUD(500)));
+            env.close();
+
+            // issuer creates two NFTs: one with and one without AutoTrustLine.
+            std::uint16_t xferFee = 5000;  // 5%
+            uint256 const nftAutoTrustID{token::getNextID(
+                env, issuer, 0u, tfTransferable | tfTrustLine, xferFee)};
+            env(token::mint(issuer, 0u),
+                token::xferFee(xferFee),
+                txflags(tfTransferable | tfTrustLine));
+            env.close();
+
+            uint256 const nftNoAutoTrustID{
+                token::getNextID(env, issuer, 0u, tfTransferable, xferFee)};
+            env(token::mint(issuer, 0u),
+                token::xferFee(xferFee),
+                txflags(tfTransferable));
+            env.close();
+
+            // becky buys the nfts for 1 drop each.
+            {
+                uint256 const beckyBuyOfferIndex1 =
+                    keylet::nftoffer(becky, env.seq(becky)).key;
+                env(token::createOffer(becky, nftAutoTrustID, drops(1)),
+                    token::owner(issuer));
+
+                uint256 const beckyBuyOfferIndex2 =
+                    keylet::nftoffer(becky, env.seq(becky)).key;
+                env(token::createOffer(becky, nftNoAutoTrustID, drops(1)),
+                    token::owner(issuer));
+
+                env.close();
+                env(token::acceptBuyOffer(issuer, beckyBuyOfferIndex1));
+                env(token::acceptBuyOffer(issuer, beckyBuyOfferIndex2));
+                env.close();
+            }
+
+            // becky creates offers to sell the nfts for AUD.
+            uint256 const beckyAutoTrustOfferIndex =
+                keylet::nftoffer(becky, env.seq(becky)).key;
+            env(token::createOffer(becky, nftAutoTrustID, gwAUD(100)),
+                txflags(tfSellNFToken));
+            env.close();
+
+            // Creating an offer for the NFToken without tfTrustLine fails
+            // because issuer does not have a trust line for AUD.
+            env(token::createOffer(becky, nftNoAutoTrustID, gwAUD(100)),
+                txflags(tfSellNFToken),
+                ter(tecNO_LINE));
+            env.close();
+
+            // issuer creates a trust line.  Now the offer create for the
+            // NFToken without tfTrustLine succeeds.
+            BEAST_EXPECT(ownerCount(env, issuer) == 0);
+            env(trust(issuer, gwAUD(1000)));
+            env.close();
+            BEAST_EXPECT(ownerCount(env, issuer) == 1);
+
+            uint256 const beckyNoAutoTrustOfferIndex =
+                keylet::nftoffer(becky, env.seq(becky)).key;
+            env(token::createOffer(becky, nftNoAutoTrustID, gwAUD(100)),
+                txflags(tfSellNFToken));
+            env.close();
+
+            // Now that the offers are in place, issuer removes the trustline.
+            BEAST_EXPECT(ownerCount(env, issuer) == 1);
+            env(trust(issuer, gwAUD(0)));
+            env.close();
+            BEAST_EXPECT(ownerCount(env, issuer) == 0);
+
+            // cheri attempts to accept becky's offers.  Behavior with the
+            // AutoTrustline NFT is uniform: issuer gets a new trust line.
+            env(token::acceptSellOffer(cheri, beckyAutoTrustOfferIndex));
+            env.close();
+
+            // Here's evidence that issuer got the new AUD trust line.
+            BEAST_EXPECT(ownerCount(env, issuer) == 1);
+            BEAST_EXPECT(env.balance(issuer, gwAUD) == gwAUD(5));
+
+            // issuer once again removes the trust line for AUD.
+            env(pay(issuer, gw, gwAUD(5)));
+            env.close();
+            BEAST_EXPECT(ownerCount(env, issuer) == 0);
+
+            // cheri attempts to accept the NoAutoTrustLine NFT.  Behavior
+            // depends on whether fixEnforceNFTokenTrustline is enabled.
+            if (feats[fixEnforceNFTokenTrustline])
+            {
+                // With fixEnforceNFTokenTrustline cheri can't accept the
+                // offer because issuer could not get their transfer fee
+                // without the appropriate trustline.
+                env(token::acceptSellOffer(cheri, beckyNoAutoTrustOfferIndex),
+                    ter(tecNO_LINE));
+                env.close();
+
+                // But if issuer re-establishes the trustline then the offer
+                // can be accepted.
+                env(trust(issuer, gwAUD(1000)));
+                env.close();
+                BEAST_EXPECT(ownerCount(env, issuer) == 1);
+
+                env(token::acceptSellOffer(cheri, beckyNoAutoTrustOfferIndex));
+                env.close();
+            }
+            else
+            {
+                // Without fixEnforceNFTokenTrustline the offer just works
+                // and issuer gets a trustline that they did not request.
+                env(token::acceptSellOffer(cheri, beckyNoAutoTrustOfferIndex));
+                env.close();
+            }
+            BEAST_EXPECT(ownerCount(env, issuer) == 1);
+            BEAST_EXPECT(env.balance(issuer, gwAUD) == gwAUD(5));
+        }  // for feats
+    }
+
     void
     testNFTIssuerIsIOUIssuer(FeatureBitset features)
     {
@@ -7609,6 +7779,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
         testFeatMintWithOffer(features);
         testTxJsonMetaFields(features);
         testFixNFTokenBuyerReserve(features);
+        testUnaskedForAutoTrustline(features);
         testNFTIssuerIsIOUIssuer(features);
     }
 

src/test/app/Offer_test.cpp

@@ -5467,12 +5467,12 @@ class Offer_manual_test : public OfferBaseUtil_test
     }
 };
 
-BEAST_DEFINE_TESTSUITE_PRIO(OfferBaseUtil, tx, ripple, 4);
-BEAST_DEFINE_TESTSUITE_PRIO(OfferWOFlowCross, tx, ripple, 4);
-BEAST_DEFINE_TESTSUITE_PRIO(OfferWTakerDryOffer, tx, ripple, 4);
-BEAST_DEFINE_TESTSUITE_PRIO(OfferWOSmallQOffers, tx, ripple, 4);
-BEAST_DEFINE_TESTSUITE_PRIO(OfferWOFillOrKill, tx, ripple, 4);
-BEAST_DEFINE_TESTSUITE_PRIO(OfferAllFeatures, tx, ripple, 4);
+BEAST_DEFINE_TESTSUITE_PRIO(OfferBaseUtil, tx, ripple, 2);
+BEAST_DEFINE_TESTSUITE_PRIO(OfferWOFlowCross, tx, ripple, 2);
+BEAST_DEFINE_TESTSUITE_PRIO(OfferWTakerDryOffer, tx, ripple, 2);
+BEAST_DEFINE_TESTSUITE_PRIO(OfferWOSmallQOffers, tx, ripple, 2);
+BEAST_DEFINE_TESTSUITE_PRIO(OfferWOFillOrKill, tx, ripple, 2);
+BEAST_DEFINE_TESTSUITE_PRIO(OfferAllFeatures, tx, ripple, 2);
 BEAST_DEFINE_TESTSUITE_MANUAL_PRIO(Offer_manual, tx, ripple, 20);
 
 }  // namespace test