From 060eff0cf428a09deb7b080d2e913d6a5ea607da Mon Sep 17 00:00:00 2001 From: Pratik Mankawde Date: Wed, 29 Oct 2025 10:50:00 +0000 Subject: [PATCH] removed amendmend code Signed-off-by: Pratik Mankawde --- include/xrpl/protocol/TxFlags.h | 13 +- include/xrpl/protocol/detail/features.macro | 2 +- src/test/app/NFToken_test.cpp | 238 ++++++-------------- src/xrpld/app/tx/detail/NFTokenMint.cpp | 21 +- 4 files changed, 74 insertions(+), 200 deletions(-) diff --git a/include/xrpl/protocol/TxFlags.h b/include/xrpl/protocol/TxFlags.h index dcbc10b927..e3bae685f1 100644 --- a/include/xrpl/protocol/TxFlags.h +++ b/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 -// amendment passes we still need to keep the old behavior available. +// tfTrustLine flag as a way to prevent the attack. Since the amendment +// 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 = - ~( ~tfNFTokenMintMask | tfTrustLine); - -// if featureDynamicNFT enabled then new flag allowing mutable URI available. -constexpr std::uint32_t const tfNFTokenMintOldMaskWithMutable = - ~( ~tfNFTokenMintOldMask | tfMutable); - +// if featureDynamicNFT enabled then flag allowing mutable URI available. constexpr std::uint32_t const tfNFTokenMintMaskWithMutable = ~( ~tfNFTokenMintMask | tfMutable); diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 085456463e..8039f2e893 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -154,4 +154,4 @@ XRPL_RETIRE(FlowCross) XRPL_RETIRE(fix1513) XRPL_RETIRE(fix1515) XRPL_RETIRE(fix1543) -XRPL_FIX(fixRemoveNFTokenAutoTrustLine) +XRPL_RETIRE(fixRemoveNFTokenAutoTrustLine) diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 9edbe4652c..43972dd9ba 100644 --- a/src/test/app/NFToken_test.cpp +++ b/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 - // presence (or absence) of the fixRemoveNFTokenAutoTrustLine - // amendment. So we test both cases here. - for (auto const& tweakedFeatures : - {features - fixRemoveNFTokenAutoTrustLine, - features | fixRemoveNFTokenAutoTrustLine}) + Env env{*this, features}; + env.fund(XRP(1000), alice, 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(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}; - env.fund(XRP(1000), alice, becky, cheri, gw); + uint256 const nftNoAutoTrustID{ + 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. - 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))); + // becky buys the nft for 1 drop. + uint256 const beckyBuyOfferIndex = + keylet::nftoffer(becky, env.seq(becky)).key; + env(token::createOffer(becky, nftNoAutoTrustID, drops(1)), + token::owner(alice)); 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(token::acceptBuyOffer(alice, beckyBuyOfferIndex)); 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}) - { - uint256 const nftNoAutoTrustID{ - token::getNextID(env, alice, 0u, tfTransferable, xferFee)}; - env(token::mint(alice, 0u), - token::xferFee(xferFee), - txflags(tfTransferable)); - env.close(); + // becky attempts to sell the nft for AUD. + TER const createOfferTER = + xferFee ? TER(tecNO_LINE) : TER(tesSUCCESS); + uint256 const beckyOfferIndex = + keylet::nftoffer(becky, env.seq(becky)).key; + env(token::createOffer(becky, nftNoAutoTrustID, gwAUD(100)), + txflags(tfSellNFToken), + ter(createOfferTER)); + env.close(); - // becky buys the nft for 1 drop. - uint256 const beckyBuyOfferIndex = - keylet::nftoffer(becky, env.seq(becky)).key; - env(token::createOffer(becky, nftNoAutoTrustID, drops(1)), - token::owner(alice)); - env.close(); - env(token::acceptBuyOffer(alice, beckyBuyOfferIndex)); - 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(); - // becky attempts to sell the nft for AUD. - TER const createOfferTER = - xferFee ? TER(tecNO_LINE) : TER(tesSUCCESS); - uint256 const beckyOfferIndex = - 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(temINVALID_FLAG) - : static_cast(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)); - } + // 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% + + env(token::mint(alice, 0u), + token::xferFee(transferFee), + txflags(tfTransferable | tfTrustLine), + ter(static_cast(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 = - features - fixRemoveNFTokenAutoTrustLine; + FeatureBitset const localFeatures = features; 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 = - features - fixRemoveNFTokenAutoTrustLine; + FeatureBitset const localFeatures = features; Env env{*this, localFeatures}; env.fund(XRP(1000), issuer, becky, cheri); diff --git a/src/xrpld/app/tx/detail/NFTokenMint.cpp b/src/xrpld/app/tx/detail/NFTokenMint.cpp index dd82443fee..8b5dd476fd 100644 --- a/src/xrpld/app/tx/detail/NFTokenMint.cpp +++ b/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 - : tfNFTokenMintMask - : ctx.rules.enabled(featureDynamicNFT) ? tfNFTokenMintOldMaskWithMutable - : tfNFTokenMintOldMask; - + ctx.rules.enabled(featureDynamicNFT) ? tfNFTokenMintMaskWithMutable + : tfNFTokenMintMask; return nfTokenMintMask; }