diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 1a0d89f895..214f849558 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -77,14 +77,11 @@ XRPL_FIX (DisallowIncomingV1, Supported::yes, VoteBehavior::DefaultNo XRPL_FEATURE(XChainBridge, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(AMM, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(Clawback, Supported::yes, VoteBehavior::DefaultNo) -XRPL_FIX (NFTokenRemint, Supported::yes, VoteBehavior::DefaultNo) -XRPL_FIX (NonFungibleTokensV1_2, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (UniversalNumber, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(XRPFees, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(DisallowIncoming, 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) XRPL_FEATURE(CheckCashMakesTrustLine, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(FlowSortStrands, Supported::yes, VoteBehavior::DefaultYes) @@ -111,9 +108,6 @@ XRPL_FEATURE(Flow, Supported::yes, VoteBehavior::DefaultYe // // If a feature remains obsolete for long enough that no clients are able // to vote for it, the feature can be removed (entirely?) from the code. -XRPL_FIX (NFTokenNegOffer, Supported::yes, VoteBehavior::Obsolete) -XRPL_FIX (NFTokenDirV1, Supported::yes, VoteBehavior::Obsolete) -XRPL_FEATURE(NonFungibleTokensV1, Supported::yes, VoteBehavior::Obsolete) XRPL_FEATURE(CryptoConditionsSuite, Supported::yes, VoteBehavior::Obsolete) // The following amendments have been active for at least two years. Their @@ -137,6 +131,8 @@ XRPL_RETIRE(fix1623) XRPL_RETIRE(fix1781) XRPL_RETIRE(fixAmendmentMajorityCalc) XRPL_RETIRE(fixCheckThreading) +XRPL_RETIRE(fixNonFungibleTokensV1_2) +XRPL_RETIRE(fixNFTokenRemint) XRPL_RETIRE(fixMasterKeyAsRegularKey) XRPL_RETIRE(fixQualityUpperBound) XRPL_RETIRE(fixReducedOffersV1) @@ -150,6 +146,7 @@ XRPL_RETIRE(FeeEscalation) XRPL_RETIRE(FlowCross) XRPL_RETIRE(ImmediateOfferKilled) XRPL_RETIRE(MultiSign) +XRPL_RETIRE(NonFungibleTokensV1_1) XRPL_RETIRE(PayChan) XRPL_RETIRE(SortedDirectories) XRPL_RETIRE(TickSize) diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index bfe5182469..0bdd3f6f97 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -332,7 +332,7 @@ TRANSACTION(ttACCOUNT_DELETE, 21, AccountDelete, #endif TRANSACTION(ttNFTOKEN_MINT, 25, NFTokenMint, Delegation::delegatable, - featureNonFungibleTokensV1, + uint256{}, changeNFTCounts, ({ {sfNFTokenTaxon, soeREQUIRED}, @@ -350,7 +350,7 @@ TRANSACTION(ttNFTOKEN_MINT, 25, NFTokenMint, #endif TRANSACTION(ttNFTOKEN_BURN, 26, NFTokenBurn, Delegation::delegatable, - featureNonFungibleTokensV1, + uint256{}, changeNFTCounts, ({ {sfNFTokenID, soeREQUIRED}, @@ -363,7 +363,7 @@ TRANSACTION(ttNFTOKEN_BURN, 26, NFTokenBurn, #endif TRANSACTION(ttNFTOKEN_CREATE_OFFER, 27, NFTokenCreateOffer, Delegation::delegatable, - featureNonFungibleTokensV1, + uint256{}, noPriv, ({ {sfNFTokenID, soeREQUIRED}, @@ -379,7 +379,7 @@ TRANSACTION(ttNFTOKEN_CREATE_OFFER, 27, NFTokenCreateOffer, #endif TRANSACTION(ttNFTOKEN_CANCEL_OFFER, 28, NFTokenCancelOffer, Delegation::delegatable, - featureNonFungibleTokensV1, + uint256{}, noPriv, ({ {sfNFTokenOffers, soeREQUIRED}, @@ -391,7 +391,7 @@ TRANSACTION(ttNFTOKEN_CANCEL_OFFER, 28, NFTokenCancelOffer, #endif TRANSACTION(ttNFTOKEN_ACCEPT_OFFER, 29, NFTokenAcceptOffer, Delegation::delegatable, - featureNonFungibleTokensV1, + uint256{}, noPriv, ({ {sfNFTokenBuyOffer, soeOPTIONAL}, diff --git a/src/libxrpl/protocol/Rules.cpp b/src/libxrpl/protocol/Rules.cpp index b472b9b0f1..ec1b7ac563 100644 --- a/src/libxrpl/protocol/Rules.cpp +++ b/src/libxrpl/protocol/Rules.cpp @@ -131,17 +131,6 @@ Rules::enabled(uint256 const& feature) const { XRPL_ASSERT(impl_, "ripple::Rules::enabled : initialized"); - // The functionality of the "NonFungibleTokensV1_1" amendment is - // precisely the functionality of the following three amendments - // so if their status is ever queried individually, we inject an - // extra check here to simplify the checking elsewhere. - if (feature == featureNonFungibleTokensV1 || - feature == fixNFTokenNegOffer || feature == fixNFTokenDirV1) - { - if (impl_->enabled(featureNonFungibleTokensV1_1)) - return true; - } - return impl_->enabled(feature); } diff --git a/src/test/app/Delegate_test.cpp b/src/test/app/Delegate_test.cpp index fca0a7b249..2c0903fc54 100644 --- a/src/test/app/Delegate_test.cpp +++ b/src/test/app/Delegate_test.cpp @@ -1800,58 +1800,6 @@ class Delegate_test : public beast::unit_test::suite for (auto const& tx : txRequiredFeatures) txAmendmentEnabled(tx.first); } - - // NFTokenMint, NFTokenBurn, NFTokenCreateOffer, NFTokenCancelOffer, and - // NFTokenAcceptOffer are tested separately. Since - // featureNonFungibleTokensV1_1 includes the functionality of - // featureNonFungibleTokensV1, fixNFTokenNegOffer, and fixNFTokenDirV1, - // both featureNonFungibleTokensV1_1 and featureNonFungibleTokensV1 need - // to be disabled to block these transactions from being delegated. - { - Env env( - *this, - features - featureNonFungibleTokensV1 - - featureNonFungibleTokensV1_1); - - Account const alice{"alice"}; - Account const bob{"bob"}; - env.fund(XRP(100000), alice, bob); - env.close(); - - for (auto const tx : - {"NFTokenMint", - "NFTokenBurn", - "NFTokenCreateOffer", - "NFTokenCancelOffer", - "NFTokenAcceptOffer"}) - { - env(delegate::set(alice, bob, {tx}), ter(temMALFORMED)); - } - } - - // NFTokenMint, NFTokenBurn, NFTokenCreateOffer, NFTokenCancelOffer, and - // NFTokenAcceptOffer are allowed to be delegated if either - // featureNonFungibleTokensV1 or featureNonFungibleTokensV1_1 is - // enabled. - { - for (auto const feature : - {featureNonFungibleTokensV1, featureNonFungibleTokensV1_1}) - { - Env env(*this, features - feature); - Account const alice{"alice"}; - Account const bob{"bob"}; - env.fund(XRP(100000), alice, bob); - env.close(); - - for (auto const tx : - {"NFTokenMint", - "NFTokenBurn", - "NFTokenCreateOffer", - "NFTokenCancelOffer", - "NFTokenAcceptOffer"}) - env(delegate::set(alice, bob, {tx})); - } - } } void diff --git a/src/test/app/FixNFTokenPageLinks_test.cpp b/src/test/app/FixNFTokenPageLinks_test.cpp index 4acd650a08..5c810169e6 100644 --- a/src/test/app/FixNFTokenPageLinks_test.cpp +++ b/src/test/app/FixNFTokenPageLinks_test.cpp @@ -69,12 +69,10 @@ class FixNFTokenPageLinks_test : public beast::unit_test::suite return 0u; }(); - // If fixNFTokenRemint amendment is on, we must - // add FirstNFTokenSequence. - if (env.current()->rules().enabled(fixNFTokenRemint)) - tokenSeq += env.le(acct) - ->at(~sfFirstNFTokenSequence) - .value_or(env.seq(acct)); + // We must add FirstNFTokenSequence. + tokenSeq += env.le(acct) + ->at(~sfFirstNFTokenSequence) + .value_or(env.seq(acct)); return toUInt32(nft::cipheredTaxon(tokenSeq, nft::toTaxon(taxon))); }; diff --git a/src/test/app/NFTokenBurn_test.cpp b/src/test/app/NFTokenBurn_test.cpp index 7e582446ef..75c88a06c9 100644 --- a/src/test/app/NFTokenBurn_test.cpp +++ b/src/test/app/NFTokenBurn_test.cpp @@ -28,7 +28,7 @@ namespace ripple { -class NFTokenBurnBaseUtil_test : public beast::unit_test::suite +class NFTokenBurn_test : public beast::unit_test::suite { // Helper function that returns the number of nfts owned by an account. static std::uint32_t @@ -375,12 +375,10 @@ class NFTokenBurnBaseUtil_test : public beast::unit_test::suite std::uint32_t tokenSeq = env.le(acct)->at(~sfMintedNFTokens).value_or(0); - // If fixNFTokenRemint amendment is on, we must - // add FirstNFTokenSequence. - if (env.current()->rules().enabled(fixNFTokenRemint)) - tokenSeq += env.le(acct) - ->at(~sfFirstNFTokenSequence) - .value_or(env.seq(acct)); + // We must add FirstNFTokenSequence. + tokenSeq += env.le(acct) + ->at(~sfFirstNFTokenSequence) + .value_or(env.seq(acct)); return toUInt32( nft::cipheredTaxon(tokenSeq, nft::toTaxon(taxon))); @@ -893,107 +891,9 @@ class NFTokenBurnBaseUtil_test : public beast::unit_test::suite using namespace test::jtx; - // Test what happens if a NFT is unburnable when there are - // more than 500 offers, before fixNonFungibleTokensV1_2 goes live - if (!features[fixNonFungibleTokensV1_2]) - { - Env env{*this, features}; - - Account const alice("alice"); - Account const becky("becky"); - env.fund(XRP(1000), alice, becky); - env.close(); - - // We structure the test to try and maximize the metadata produced. - // This verifies that we don't create too much metadata during a - // maximal burn operation. - // - // 1. alice mints an nft with a full-sized URI. - // 2. We create 500 new accounts, each of which creates an offer - // for alice's nft. - // 3. becky creates one more offer for alice's NFT - // 4. Attempt to burn the nft which fails because there are too - // many offers. - // 5. Cancel becky's offer and the nft should become burnable. - uint256 const nftokenID = - token::getNextID(env, alice, 0, tfTransferable); - env(token::mint(alice, 0), - token::uri(std::string(maxTokenURILength, 'u')), - txflags(tfTransferable)); - env.close(); - - std::vector offerIndexes; - offerIndexes.reserve(maxTokenOfferCancelCount); - for (std::uint32_t i = 0; i < maxTokenOfferCancelCount; ++i) - { - Account const acct(std::string("acct") + std::to_string(i)); - env.fund(XRP(1000), acct); - env.close(); - - offerIndexes.push_back( - keylet::nftoffer(acct, env.seq(acct)).key); - env(token::createOffer(acct, nftokenID, drops(1)), - token::owner(alice)); - env.close(); - } - - // Verify all offers are present in the ledger. - for (uint256 const& offerIndex : offerIndexes) - { - BEAST_EXPECT(env.le(keylet::nftoffer(offerIndex))); - } - - // Create one too many offers. - uint256 const beckyOfferIndex = - keylet::nftoffer(becky, env.seq(becky)).key; - env(token::createOffer(becky, nftokenID, drops(1)), - token::owner(alice)); - - // Attempt to burn the nft which should fail. - env(token::burn(alice, nftokenID), ter(tefTOO_BIG)); - - // Close enough ledgers that the burn transaction is no longer - // retried. - for (int i = 0; i < 10; ++i) - env.close(); - - // Cancel becky's offer, but alice adds a sell offer. The token - // should still not be burnable. - env(token::cancelOffer(becky, {beckyOfferIndex})); - env.close(); - - uint256 const aliceOfferIndex = - keylet::nftoffer(alice, env.seq(alice)).key; - env(token::createOffer(alice, nftokenID, drops(1)), - txflags(tfSellNFToken)); - env.close(); - - env(token::burn(alice, nftokenID), ter(tefTOO_BIG)); - env.close(); - - // Cancel alice's sell offer. Now the token should be burnable. - env(token::cancelOffer(alice, {aliceOfferIndex})); - env.close(); - - env(token::burn(alice, nftokenID)); - env.close(); - - // Burning the token should remove all the offers from the ledger. - for (uint256 const& offerIndex : offerIndexes) - { - BEAST_EXPECT(!env.le(keylet::nftoffer(offerIndex))); - } - - // Both alice and becky should have ownerCounts of zero. - BEAST_EXPECT(ownerCount(env, alice) == 0); - BEAST_EXPECT(ownerCount(env, becky) == 0); - } - // Test that up to 499 buy/sell offers will be removed when NFT is - // burned after fixNonFungibleTokensV1_2 is enabled. This is to test - // that we can successfully remove all offers if the number of offers is - // less than 500. - if (features[fixNonFungibleTokensV1_2]) + // burned. This is to test that we can successfully remove all offers + // if the number of offers is less than 500. { Env env{*this, features}; @@ -1042,9 +942,7 @@ class NFTokenBurnBaseUtil_test : public beast::unit_test::suite BEAST_EXPECT(ownerCount(env, becky) == 0); } - // Test that up to 500 buy offers are removed when NFT is burned - // after fixNonFungibleTokensV1_2 is enabled - if (features[fixNonFungibleTokensV1_2]) + // Test that up to 500 buy offers are removed when NFT is burned. { Env env{*this, features}; @@ -1087,9 +985,7 @@ class NFTokenBurnBaseUtil_test : public beast::unit_test::suite BEAST_EXPECT(ownerCount(env, alice) == 1); } - // Test that up to 500 buy/sell offers are removed when NFT is burned - // after fixNonFungibleTokensV1_2 is enabled - if (features[fixNonFungibleTokensV1_2]) + // Test that up to 500 buy/sell offers are removed when NFT is burned. { Env env{*this, features}; @@ -1181,12 +1077,10 @@ class NFTokenBurnBaseUtil_test : public beast::unit_test::suite std::uint32_t tokenSeq = env.le(acct)->at(~sfMintedNFTokens).value_or(0); - // If fixNFTokenRemint amendment is on, we must - // add FirstNFTokenSequence. - if (env.current()->rules().enabled(fixNFTokenRemint)) - tokenSeq += env.le(acct) - ->at(~sfFirstNFTokenSequence) - .value_or(env.seq(acct)); + // We must add FirstNFTokenSequence. + tokenSeq += env.le(acct) + ->at(~sfFirstNFTokenSequence) + .value_or(env.seq(acct)); return toUInt32( nft::cipheredTaxon(tokenSeq, nft::toTaxon(taxon))); @@ -1363,6 +1257,9 @@ class NFTokenBurnBaseUtil_test : public beast::unit_test::suite } } +protected: + FeatureBitset const allFeatures{test::jtx::testable_amendments()}; + void testWithFeats(FeatureBitset features) { @@ -1372,84 +1269,15 @@ class NFTokenBurnBaseUtil_test : public beast::unit_test::suite exerciseBrokenLinks(features); } -protected: - void - run(std::uint32_t instance, bool last = false) - { - using namespace test::jtx; - static FeatureBitset const all{testable_amendments()}; - static FeatureBitset const fixNFTV1_2{fixNonFungibleTokensV1_2}; - static FeatureBitset const fixNFTDir{fixNFTokenDirV1}; - static FeatureBitset const fixNFTRemint{fixNFTokenRemint}; - static FeatureBitset const fixNFTPageLinks{fixNFTokenPageLinks}; - - static std::array const feats{ - all - fixNFTV1_2 - fixNFTDir - fixNFTRemint - fixNFTPageLinks, - all - fixNFTV1_2 - fixNFTRemint - fixNFTPageLinks, - all - fixNFTRemint - fixNFTPageLinks, - all - fixNFTPageLinks, - all, - }; - - if (BEAST_EXPECT(instance < feats.size())) - { - testWithFeats(feats[instance]); - } - BEAST_EXPECT(!last || instance == feats.size() - 1); - } - public: void run() override { - run(0); + testWithFeats(allFeatures - fixNFTokenPageLinks); + testWithFeats(allFeatures); } }; -class NFTokenBurnWOfixFungTokens_test : public NFTokenBurnBaseUtil_test -{ -public: - void - run() override - { - NFTokenBurnBaseUtil_test::run(1); - } -}; - -class NFTokenBurnWOFixTokenRemint_test : public NFTokenBurnBaseUtil_test -{ -public: - void - run() override - { - NFTokenBurnBaseUtil_test::run(2); - } -}; - -class NFTokenBurnWOFixNFTPageLinks_test : public NFTokenBurnBaseUtil_test -{ -public: - void - run() override - { - NFTokenBurnBaseUtil_test::run(3); - } -}; - -class NFTokenBurnAllFeatures_test : public NFTokenBurnBaseUtil_test -{ -public: - void - run() override - { - NFTokenBurnBaseUtil_test::run(4, true); - } -}; - -BEAST_DEFINE_TESTSUITE_PRIO(NFTokenBurnBaseUtil, app, ripple, 3); -BEAST_DEFINE_TESTSUITE_PRIO(NFTokenBurnWOfixFungTokens, app, ripple, 3); -BEAST_DEFINE_TESTSUITE_PRIO(NFTokenBurnWOFixTokenRemint, app, ripple, 3); -BEAST_DEFINE_TESTSUITE_PRIO(NFTokenBurnWOFixNFTPageLinks, app, ripple, 3); -BEAST_DEFINE_TESTSUITE_PRIO(NFTokenBurnAllFeatures, app, ripple, 3); +BEAST_DEFINE_TESTSUITE_PRIO(NFTokenBurn, app, ripple, 3); } // namespace ripple diff --git a/src/test/app/NFTokenDir_test.cpp b/src/test/app/NFTokenDir_test.cpp index 19f4f7efba..0e3dca361c 100644 --- a/src/test/app/NFTokenDir_test.cpp +++ b/src/test/app/NFTokenDir_test.cpp @@ -191,11 +191,10 @@ class NFTokenDir_test : public beast::unit_test::suite Account::base58Seed, std::string(seed)); env.fund(XRP(10000), account); - // Do not close the ledger inside the loop. If - // fixNFTokenRemint is enabled and accounts are initialized - // at different ledgers, they will have different account - // sequences. That would cause the accounts to have - // different NFTokenID sequence numbers. + // Do not close the ledger inside the loop. If accounts are + // initialized at different ledgers, they will have + // different account sequences. That would cause the + // accounts to have different NFTokenID sequence numbers. } env.close(); @@ -377,11 +376,9 @@ class NFTokenDir_test : public beast::unit_test::suite } void - testFixNFTokenDirV1(FeatureBitset features) + testNFTokenDir(FeatureBitset features) { - // Exercise a fix for an off-by-one in the creation of an NFTokenPage - // index. - testcase("fixNFTokenDirV1"); + testcase("NFTokenDir"); using namespace test::jtx; @@ -391,7 +388,7 @@ class NFTokenDir_test : public beast::unit_test::suite // the index for the new page. This test recreates the problem. // Lambda that exercises the split. - auto exerciseFixNFTokenDirV1 = + auto exercise = [this, &features](std::initializer_list seeds) { Env env{ @@ -415,11 +412,10 @@ class NFTokenDir_test : public beast::unit_test::suite Account::base58Seed, std::string(seed)); env.fund(XRP(10000), account); - // Do not close the ledger inside the loop. If - // fixNFTokenRemint is enabled and accounts are initialized - // at different ledgers, they will have different account - // sequences. That would cause the accounts to have - // different NFTokenID sequence numbers. + // Do not close the ledger inside the loop. If accounts are + // initialized at different ledgers, they will have + // different account sequences. That would cause the + // accounts to have different NFTokenID sequence numbers. } env.close(); @@ -453,16 +449,6 @@ class NFTokenDir_test : public beast::unit_test::suite env.close(); } - // Here is the last offer. Without the fix accepting this - // offer causes tecINVARIANT_FAILED. With the fix the offer - // accept succeeds. - if (!features[fixNFTokenDirV1]) - { - env(token::acceptSellOffer(buyer, offers.back()), - ter(tecINVARIANT_FAILED)); - env.close(); - return; - } env(token::acceptSellOffer(buyer, offers.back())); env.close(); @@ -598,8 +584,8 @@ class NFTokenDir_test : public beast::unit_test::suite }; // Run the test cases. - exerciseFixNFTokenDirV1(seventeenHi); - exerciseFixNFTokenDirV1(seventeenLo); + exercise(seventeenHi); + exercise(seventeenLo); } void @@ -665,10 +651,9 @@ class NFTokenDir_test : public beast::unit_test::suite accounts.emplace_back(Account::base58Seed, std::string(seed)); env.fund(XRP(10000), account); - // Do not close the ledger inside the loop. If - // fixNFTokenRemint is enabled and accounts are initialized - // at different ledgers, they will have different account - // sequences. That would cause the accounts to have + // Do not close the ledger inside the loop. If accounts are + // initialized at different ledgers, they will have different + // account sequences. That would cause the accounts to have // different NFTokenID sequence numbers. } env.close(); @@ -760,11 +745,7 @@ class NFTokenDir_test : public beast::unit_test::suite // All NFTs should now be accounted for, so nftIDs should be empty. BEAST_EXPECT(nftIDs.empty()); - // Show that Without fixNFTokenDirV1 no more NFTs can be added to - // buyer. Also show that fixNFTokenDirV1 fixes the problem. - TER const expect = features[fixNFTokenDirV1] - ? static_cast(tesSUCCESS) - : static_cast(tecNO_SUITABLE_NFTOKEN_PAGE); + TER const expect = tesSUCCESS; env(token::mint(buyer, 0), txflags(tfTransferable), ter(expect)); env.close(); } @@ -783,11 +764,6 @@ class NFTokenDir_test : public beast::unit_test::suite // Lastly, none of the remaining NFTs should be acquirable by the // buyer. They would cause page overflow. - // This test collapses in a heap if fixNFTokenDirV1 is not enabled. - // If it is enabled just return so we skip the test. - if (!features[fixNFTokenDirV1]) - return; - testcase("NFToken consecutive packing"); using namespace test::jtx; @@ -846,10 +822,9 @@ class NFTokenDir_test : public beast::unit_test::suite accounts.emplace_back(Account::base58Seed, std::string(seed)); env.fund(XRP(10000), account); - // Do not close the ledger inside the loop. If - // fixNFTokenRemint is enabled and accounts are initialized - // at different ledgers, they will have different account - // sequences. That would cause the accounts to have + // Do not close the ledger inside the loop. If accounts are + // initialized at different ledgers, they will have different + // account sequences. That would cause the accounts to have // different NFTokenID sequence numbers. } env.close(); @@ -1088,7 +1063,7 @@ class NFTokenDir_test : public beast::unit_test::suite { testConsecutiveNFTs(features); testLopsidedSplits(features); - testFixNFTokenDirV1(features); + testNFTokenDir(features); testTooManyEquivalent(features); testConsecutivePacking(features); } @@ -1099,11 +1074,7 @@ public: { using namespace test::jtx; FeatureBitset const all{testable_amendments()}; - FeatureBitset const fixNFTDir{ - fixNFTokenDirV1, featureNonFungibleTokensV1_1}; - testWithFeats(all - fixNFTDir - fixNFTokenRemint); - testWithFeats(all - fixNFTokenRemint); testWithFeats(all); } }; diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 9edbe4652c..7d7774c15c 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -87,52 +87,6 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite testcase("Enabled"); using namespace test::jtx; - { - // If the NFT amendment is not enabled, you should not be able - // to create or burn NFTs. - Env env{ - *this, - features - featureNonFungibleTokensV1 - - featureNonFungibleTokensV1_1}; - Account const& master = env.master; - - BEAST_EXPECT(ownerCount(env, master) == 0); - BEAST_EXPECT(mintedCount(env, master) == 0); - BEAST_EXPECT(burnedCount(env, master) == 0); - - uint256 const nftId{token::getNextID(env, master, 0u)}; - env(token::mint(master, 0u), ter(temDISABLED)); - env.close(); - BEAST_EXPECT(ownerCount(env, master) == 0); - BEAST_EXPECT(mintedCount(env, master) == 0); - BEAST_EXPECT(burnedCount(env, master) == 0); - - env(token::burn(master, nftId), ter(temDISABLED)); - env.close(); - BEAST_EXPECT(ownerCount(env, master) == 0); - BEAST_EXPECT(mintedCount(env, master) == 0); - BEAST_EXPECT(burnedCount(env, master) == 0); - - uint256 const offerIndex = - keylet::nftoffer(master, env.seq(master)).key; - env(token::createOffer(master, nftId, XRP(10)), ter(temDISABLED)); - env.close(); - BEAST_EXPECT(ownerCount(env, master) == 0); - BEAST_EXPECT(mintedCount(env, master) == 0); - BEAST_EXPECT(burnedCount(env, master) == 0); - - env(token::cancelOffer(master, {offerIndex}), ter(temDISABLED)); - env.close(); - BEAST_EXPECT(ownerCount(env, master) == 0); - BEAST_EXPECT(mintedCount(env, master) == 0); - BEAST_EXPECT(burnedCount(env, master) == 0); - - env(token::acceptBuyOffer(master, offerIndex), ter(temDISABLED)); - env.close(); - BEAST_EXPECT(ownerCount(env, master) == 0); - BEAST_EXPECT(mintedCount(env, master) == 0); - BEAST_EXPECT(burnedCount(env, master) == 0); - } { // If the NFT amendment is enabled all NFT-related // facilities should be available. @@ -481,7 +435,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite // checks with this modify() call. If you call close() between // here and the end of the test all the effort will be lost. env.app().openLedger().modify( - [&alice, &env](OpenView& view, beast::Journal j) { + [&alice](OpenView& view, beast::Journal j) { // Get the account root we want to hijack. auto const sle = view.read(keylet::account(alice.id())); if (!sle) @@ -493,23 +447,13 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite if (replacement->getFieldU32(sfMintedNFTokens) != 1) return false; // Unexpected test conditions. - if (env.current()->rules().enabled(fixNFTokenRemint)) - { - // If fixNFTokenRemint is enabled, sequence number is - // generated by sfFirstNFTokenSequence + sfMintedNFTokens. - // We can replace the two fields with any numbers as long as - // they add up to the largest valid number. In our case, - // sfFirstNFTokenSequence is set to the largest valid - // number, and sfMintedNFTokens is set to zero. - (*replacement)[sfFirstNFTokenSequence] = 0xFFFF'FFFE; - (*replacement)[sfMintedNFTokens] = 0x0000'0000; - } - else - { - // Now replace sfMintedNFTokens with the largest valid - // value. - (*replacement)[sfMintedNFTokens] = 0xFFFF'FFFE; - } + // Wequence number is generated by sfFirstNFTokenSequence + + // sfMintedNFTokens. We can replace the two fields with any + // numbers as long as they add up to the largest valid number. + // In our case, sfFirstNFTokenSequence is set to the largest + // valid number, and sfMintedNFTokens is set to zero. + (*replacement)[sfFirstNFTokenSequence] = 0xFFFF'FFFE; + (*replacement)[sfMintedNFTokens] = 0x0000'0000; view.rawReplace(replacement); return true; }); @@ -2908,12 +2852,9 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite { // issuer cannot broker the offers, because they are not the // Destination. - TER const expectTer = features[fixNonFungibleTokensV1_2] - ? tecNO_PERMISSION - : tecNFTOKEN_BUY_SELL_MISMATCH; env(token::brokerOffers( issuer, offerBuyerToMinter, offerMinterToBroker), - ter(expectTer)); + ter(tecNO_PERMISSION)); env.close(); BEAST_EXPECT(ownerCount(env, issuer) == 0); BEAST_EXPECT(ownerCount(env, minter) == 2); @@ -2958,31 +2899,22 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite { // Cannot broker offers when the sell destination is not the // buyer. - TER const expectTer = features[fixNonFungibleTokensV1_2] - ? tecNO_PERMISSION - : tecNFTOKEN_BUY_SELL_MISMATCH; env(token::brokerOffers( broker, offerIssuerToBuyer, offerBuyerToMinter), - ter(expectTer)); + ter(tecNO_PERMISSION)); env.close(); BEAST_EXPECT(ownerCount(env, issuer) == 1); BEAST_EXPECT(ownerCount(env, minter) == 1); BEAST_EXPECT(ownerCount(env, buyer) == 2); - // amendment switch: When enabled the broker fails, when - // disabled the broker succeeds if the destination is the buyer. - TER const eexpectTer = features[fixNonFungibleTokensV1_2] - ? tecNO_PERMISSION - : TER(tesSUCCESS); env(token::brokerOffers( broker, offerMinterToBuyer, offerBuyerToMinter), - ter(eexpectTer)); + ter(tecNO_PERMISSION)); env.close(); - if (features[fixNonFungibleTokensV1_2]) - // Buyer is successful with acceptOffer. - env(token::acceptBuyOffer(buyer, offerMinterToBuyer)); + // Buyer is successful with acceptOffer. + env(token::acceptBuyOffer(buyer, offerMinterToBuyer)); env.close(); // Clean out the unconsumed offer. @@ -3021,12 +2953,9 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite { // Cannot broker offers when the sell destination is not the // buyer or the broker. - TER const expectTer = features[fixNonFungibleTokensV1_2] - ? tecNO_PERMISSION - : tecNFTOKEN_BUY_SELL_MISMATCH; env(token::brokerOffers( issuer, offerBuyerToBroker, offerMinterToBroker), - ter(expectTer)); + ter(tecNO_PERMISSION)); env.close(); BEAST_EXPECT(ownerCount(env, issuer) == 0); BEAST_EXPECT(ownerCount(env, minter) == 2); @@ -3907,11 +3836,8 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite using namespace test::jtx; - for (auto const& tweakedFeatures : - {features - fixNonFungibleTokensV1_2, - features | fixNonFungibleTokensV1_2}) { - Env env{*this, tweakedFeatures}; + Env env{*this, features}; auto const baseFee = env.current()->fees().base; // The most important thing to explore here is the way funds are @@ -4447,45 +4373,22 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite token::owner(minter)); env.close(); - if (tweakedFeatures[fixNonFungibleTokensV1_2]) - { - env(token::brokerOffers( - broker, buyOfferIndex, minterOfferIndex), - token::brokerFee(gwXAU(50))); - env.close(); - BEAST_EXPECT(ownerCount(env, issuer) == 1); - BEAST_EXPECT(ownerCount(env, minter) == 1); - BEAST_EXPECT(ownerCount(env, buyer) == 2); - BEAST_EXPECT(ownerCount(env, broker) == 1); - BEAST_EXPECT(env.balance(issuer, gwXAU) == gwXAU(1237.5)); - BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(1712.5)); - BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(0)); - BEAST_EXPECT(env.balance(broker, gwXAU) == gwXAU(550)); + env(token::brokerOffers( + broker, buyOfferIndex, minterOfferIndex), + token::brokerFee(gwXAU(50))); + env.close(); + BEAST_EXPECT(ownerCount(env, issuer) == 1); + BEAST_EXPECT(ownerCount(env, minter) == 1); + BEAST_EXPECT(ownerCount(env, buyer) == 2); + BEAST_EXPECT(ownerCount(env, broker) == 1); + BEAST_EXPECT(env.balance(issuer, gwXAU) == gwXAU(1237.5)); + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(1712.5)); + BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(0)); + BEAST_EXPECT(env.balance(broker, gwXAU) == gwXAU(550)); - // Burn the NFT so the next test starts with a clean state. - env(token::burn(buyer, nftID)); - env.close(); - } - else - { - env(token::brokerOffers( - broker, buyOfferIndex, minterOfferIndex), - token::brokerFee(gwXAU(50)), - ter(tecINSUFFICIENT_FUNDS)); - env.close(); - BEAST_EXPECT(ownerCount(env, issuer) == 1); - BEAST_EXPECT(ownerCount(env, minter) == 3); - BEAST_EXPECT(ownerCount(env, buyer) == 2); - BEAST_EXPECT(ownerCount(env, broker) == 1); - BEAST_EXPECT(env.balance(issuer, gwXAU) == gwXAU(1000)); - BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(1000)); - BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(1000)); - BEAST_EXPECT(env.balance(broker, gwXAU) == gwXAU(500)); - - // Burn the NFT so the next test starts with a clean state. - env(token::burn(minter, nftID)); - env.close(); - } + // Burn the NFT so the next test starts with a clean state. + env(token::burn(buyer, nftID)); + env.close(); } } } @@ -4980,32 +4883,19 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite } void - testFixNFTokenNegOffer(FeatureBitset features) + testNFTokenNegOffer(FeatureBitset features) { - // Exercise changes introduced by fixNFTokenNegOffer. using namespace test::jtx; - testcase("fixNFTokenNegOffer"); + testcase("NFTokenNegOffer"); Account const issuer{"issuer"}; Account const buyer{"buyer"}; Account const gw{"gw"}; IOU const gwXAU(gw["XAU"]); - // Test both with and without fixNFTokenNegOffer and - // fixNonFungibleTokensV1_2. Need to turn off fixNonFungibleTokensV1_2 - // as well because that amendment came later and addressed the - // acceptance side of this issue. - for (auto const& tweakedFeatures : - {features - fixNFTokenNegOffer - featureNonFungibleTokensV1_1 - - fixNonFungibleTokensV1_2, - features - fixNFTokenNegOffer - featureNonFungibleTokensV1_1, - features | fixNFTokenNegOffer}) { - // There was a bug in the initial NFT implementation that - // allowed offers to be placed with negative amounts. Verify - // that fixNFTokenNegOffer addresses the problem. - Env env{*this, tweakedFeatures}; + Env env{*this, features}; env.fund(XRP(1000000), issuer, buyer, gw); env.close(); @@ -5030,9 +4920,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite env(token::mint(issuer, 1), txflags(tfTransferable)); env.close(); - TER const offerCreateTER = tweakedFeatures[fixNFTokenNegOffer] - ? static_cast(temBAD_AMOUNT) - : static_cast(tesSUCCESS); + TER const offerCreateTER = temBAD_AMOUNT; // Make offers with negative amounts for the NFTs uint256 const sellNegXrpOfferIndex = @@ -5065,11 +4953,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite { // Now try to accept the offers. - // 1. If fixNFTokenNegOffer is NOT enabled get tecINTERNAL. - // 2. If fixNFTokenNegOffer IS enabled get tecOBJECT_NOT_FOUND. - TER const offerAcceptTER = tweakedFeatures[fixNFTokenNegOffer] - ? static_cast(tecOBJECT_NOT_FOUND) - : static_cast(tecINTERNAL); + TER const offerAcceptTER = tecOBJECT_NOT_FOUND; // Sell offers. env(token::acceptSellOffer(buyer, sellNegXrpOfferIndex), @@ -5088,13 +4972,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite env.close(); } { - // 1. If fixNFTokenNegOffer is enabled get tecOBJECT_NOT_FOUND - // 2. If it is not enabled, but fixNonFungibleTokensV1_2 is - // enabled, get tecOBJECT_NOT_FOUND. - // 3. If neither are enabled, get tesSUCCESS. - TER const offerAcceptTER = tweakedFeatures[fixNFTokenNegOffer] - ? static_cast(tecOBJECT_NOT_FOUND) - : static_cast(tesSUCCESS); + TER const offerAcceptTER = tecOBJECT_NOT_FOUND; // Brokered offers. env(token::brokerOffers( @@ -5108,100 +4986,9 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite } } - // Test what happens if NFTokenOffers are created with negative amounts - // and then fixNFTokenNegOffer goes live. What does an acceptOffer do? { - Env env{ - *this, - features - fixNFTokenNegOffer - featureNonFungibleTokensV1_1}; - - env.fund(XRP(1000000), issuer, buyer, gw); - env.close(); - - env(trust(issuer, gwXAU(2000))); - env(trust(buyer, gwXAU(2000))); - env.close(); - - env(pay(gw, issuer, gwXAU(1000))); - env(pay(gw, buyer, gwXAU(1000))); - env.close(); - - // Create an NFT that we'll make XRP offers for. - uint256 const nftID0{ - token::getNextID(env, issuer, 0u, tfTransferable)}; - env(token::mint(issuer, 0), txflags(tfTransferable)); - env.close(); - - // Create an NFT that we'll make IOU offers for. - uint256 const nftID1{ - token::getNextID(env, issuer, 1u, tfTransferable)}; - env(token::mint(issuer, 1), txflags(tfTransferable)); - env.close(); - - // Make offers with negative amounts for the NFTs - uint256 const sellNegXrpOfferIndex = - keylet::nftoffer(issuer, env.seq(issuer)).key; - env(token::createOffer(issuer, nftID0, XRP(-2)), - txflags(tfSellNFToken)); - env.close(); - - uint256 const sellNegIouOfferIndex = - keylet::nftoffer(issuer, env.seq(issuer)).key; - env(token::createOffer(issuer, nftID1, gwXAU(-2)), - txflags(tfSellNFToken)); - env.close(); - - uint256 const buyNegXrpOfferIndex = - keylet::nftoffer(buyer, env.seq(buyer)).key; - env(token::createOffer(buyer, nftID0, XRP(-1)), - token::owner(issuer)); - env.close(); - - uint256 const buyNegIouOfferIndex = - keylet::nftoffer(buyer, env.seq(buyer)).key; - env(token::createOffer(buyer, nftID1, gwXAU(-1)), - token::owner(issuer)); - env.close(); - - // Now the amendment passes. - env.enableFeature(fixNFTokenNegOffer); - env.close(); - - // All attempts to accept the offers with negative amounts - // should fail with temBAD_OFFER. - env(token::acceptSellOffer(buyer, sellNegXrpOfferIndex), - ter(temBAD_OFFER)); - env.close(); - env(token::acceptSellOffer(buyer, sellNegIouOfferIndex), - ter(temBAD_OFFER)); - env.close(); - - // Buy offers. - env(token::acceptBuyOffer(issuer, buyNegXrpOfferIndex), - ter(temBAD_OFFER)); - env.close(); - env(token::acceptBuyOffer(issuer, buyNegIouOfferIndex), - ter(temBAD_OFFER)); - env.close(); - - // Brokered offers. - env(token::brokerOffers( - gw, buyNegXrpOfferIndex, sellNegXrpOfferIndex), - ter(temBAD_OFFER)); - env.close(); - env(token::brokerOffers( - gw, buyNegIouOfferIndex, sellNegIouOfferIndex), - ter(temBAD_OFFER)); - env.close(); - } - - // Test buy offers with a destination with and without - // fixNFTokenNegOffer. - for (auto const& tweakedFeatures : - {features - fixNFTokenNegOffer - featureNonFungibleTokensV1_1, - features | fixNFTokenNegOffer}) - { - Env env{*this, tweakedFeatures}; + // Test buy offers with a destination. + Env env{*this, features}; env.fund(XRP(1000000), issuer, buyer); @@ -5211,9 +4998,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite env(token::mint(issuer, 0), txflags(tfTransferable)); env.close(); - TER const offerCreateTER = tweakedFeatures[fixNFTokenNegOffer] - ? static_cast(tesSUCCESS) - : static_cast(temMALFORMED); + TER const offerCreateTER = tesSUCCESS; env(token::createOffer(buyer, nftID, drops(1)), token::owner(issuer), @@ -5230,11 +5015,8 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite testcase("Payments with IOU transfer fees"); - for (auto const& tweakedFeatures : - {features - fixNonFungibleTokensV1_2, - features | fixNonFungibleTokensV1_2}) { - Env env{*this, tweakedFeatures}; + Env env{*this, features}; Account const minter{"minter"}; Account const secondarySeller{"seller"}; @@ -5384,22 +5166,11 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite auto const nftID = mintNFT(minter); auto const offerID = createSellOffer(minter, nftID, gwXAU(1000)); - auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] - ? static_cast(tecINSUFFICIENT_FUNDS) - : static_cast(tesSUCCESS); + TER const sellTER = tecINSUFFICIENT_FUNDS; env(token::acceptSellOffer(buyer, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixNonFungibleTokensV1_2]) - expectInitialState(); - else - { - BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(1000)); - BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(-20)); - BEAST_EXPECT( - env.balance(gw, minter["XAU"]) == gwXAU(-1000)); - BEAST_EXPECT(env.balance(gw, buyer["XAU"]) == gwXAU(20)); - } + expectInitialState(); } { // Buyer attempts to send 100% of their balance of an IOU @@ -5408,22 +5179,11 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite auto const nftID = mintNFT(minter); auto const offerID = createBuyOffer(buyer, minter, nftID, gwXAU(1000)); - auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] - ? static_cast(tecINSUFFICIENT_FUNDS) - : static_cast(tesSUCCESS); + TER const sellTER = tecINSUFFICIENT_FUNDS; env(token::acceptBuyOffer(minter, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixNonFungibleTokensV1_2]) - expectInitialState(); - else - { - BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(1000)); - BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(-20)); - BEAST_EXPECT( - env.balance(gw, minter["XAU"]) == gwXAU(-1000)); - BEAST_EXPECT(env.balance(gw, buyer["XAU"]) == gwXAU(20)); - } + expectInitialState(); } { // Buyer attempts to send an amount less than 100% of their @@ -5432,21 +5192,11 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite reinitializeTrustLineBalances(); auto const nftID = mintNFT(minter); auto const offerID = createSellOffer(minter, nftID, gwXAU(995)); - auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] - ? static_cast(tecINSUFFICIENT_FUNDS) - : static_cast(tesSUCCESS); + TER const sellTER = tecINSUFFICIENT_FUNDS; env(token::acceptSellOffer(buyer, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixNonFungibleTokensV1_2]) - expectInitialState(); - else - { - BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(995)); - BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(-14.9)); - BEAST_EXPECT(env.balance(gw, minter["XAU"]) == gwXAU(-995)); - BEAST_EXPECT(env.balance(gw, buyer["XAU"]) == gwXAU(14.9)); - } + expectInitialState(); } { // Buyer attempts to send an amount less than 100% of their @@ -5456,21 +5206,11 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite auto const nftID = mintNFT(minter); auto const offerID = createBuyOffer(buyer, minter, nftID, gwXAU(995)); - auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] - ? static_cast(tecINSUFFICIENT_FUNDS) - : static_cast(tesSUCCESS); + TER const sellTER = tecINSUFFICIENT_FUNDS; env(token::acceptBuyOffer(minter, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixNonFungibleTokensV1_2]) - expectInitialState(); - else - { - BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(995)); - BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(-14.9)); - BEAST_EXPECT(env.balance(gw, minter["XAU"]) == gwXAU(-995)); - BEAST_EXPECT(env.balance(gw, buyer["XAU"]) == gwXAU(14.9)); - } + expectInitialState(); } { // Buyer attempts to send an amount less than 100% of their @@ -5557,20 +5297,12 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite auto const nftID = mintNFT(minter); auto const offerID = createSellOffer(minter, nftID, gwXAU(1000)); - auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] - ? static_cast(tesSUCCESS) - : static_cast(tecINSUFFICIENT_FUNDS); + TER const sellTER = tesSUCCESS; env(token::acceptSellOffer(gw, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixNonFungibleTokensV1_2]) - { - BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(1000)); - BEAST_EXPECT( - env.balance(gw, minter["XAU"]) == gwXAU(-1000)); - } - else - expectInitialState(); + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(1000)); + BEAST_EXPECT(env.balance(gw, minter["XAU"]) == gwXAU(-1000)); } { // Gateway attempts to buy NFT with their own IOU - no @@ -5578,25 +5310,15 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite reinitializeTrustLineBalances(); auto const nftID = mintNFT(minter); - auto const offerTER = tweakedFeatures[fixNonFungibleTokensV1_2] - ? static_cast(tesSUCCESS) - : static_cast(tecUNFUNDED_OFFER); + TER const offerTER = tesSUCCESS; auto const offerID = createBuyOffer(gw, minter, nftID, gwXAU(1000), {offerTER}); - auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] - ? static_cast(tesSUCCESS) - : static_cast(tecOBJECT_NOT_FOUND); + TER const sellTER = tesSUCCESS; env(token::acceptBuyOffer(minter, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixNonFungibleTokensV1_2]) - { - BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(1000)); - BEAST_EXPECT( - env.balance(gw, minter["XAU"]) == gwXAU(-1000)); - } - else - expectInitialState(); + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(1000)); + BEAST_EXPECT(env.balance(gw, minter["XAU"]) == gwXAU(-1000)); } { // Gateway attempts to buy NFT with their own IOU for more @@ -5605,20 +5327,12 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite auto const nftID = mintNFT(minter); auto const offerID = createSellOffer(minter, nftID, gwXAU(5000)); - auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] - ? static_cast(tesSUCCESS) - : static_cast(tecINSUFFICIENT_FUNDS); + TER const sellTER = tesSUCCESS; env(token::acceptSellOffer(gw, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixNonFungibleTokensV1_2]) - { - BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(5000)); - BEAST_EXPECT( - env.balance(gw, minter["XAU"]) == gwXAU(-5000)); - } - else - expectInitialState(); + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(5000)); + BEAST_EXPECT(env.balance(gw, minter["XAU"]) == gwXAU(-5000)); } { // Gateway attempts to buy NFT with their own IOU for more @@ -5626,25 +5340,15 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite reinitializeTrustLineBalances(); auto const nftID = mintNFT(minter); - auto const offerTER = tweakedFeatures[fixNonFungibleTokensV1_2] - ? static_cast(tesSUCCESS) - : static_cast(tecUNFUNDED_OFFER); + TER const offerTER = tesSUCCESS; auto const offerID = createBuyOffer(gw, minter, nftID, gwXAU(5000), {offerTER}); - auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] - ? static_cast(tesSUCCESS) - : static_cast(tecOBJECT_NOT_FOUND); + TER const sellTER = tesSUCCESS; env(token::acceptBuyOffer(minter, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixNonFungibleTokensV1_2]) - { - BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(5000)); - BEAST_EXPECT( - env.balance(gw, minter["XAU"]) == gwXAU(-5000)); - } - else - expectInitialState(); + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(5000)); + BEAST_EXPECT(env.balance(gw, minter["XAU"]) == gwXAU(-5000)); } { // Gateway is the NFT minter and attempts to sell NFT for an @@ -5779,25 +5483,11 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite // now we can do a secondary sale auto const offerID = createSellOffer(secondarySeller, nftID, gwXAU(1000)); - auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] - ? static_cast(tecINSUFFICIENT_FUNDS) - : static_cast(tesSUCCESS); + TER const sellTER = tecINSUFFICIENT_FUNDS; env(token::acceptSellOffer(buyer, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixNonFungibleTokensV1_2]) - expectInitialState(); - else - { - BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(30)); - BEAST_EXPECT( - env.balance(secondarySeller, gwXAU) == gwXAU(970)); - BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(-20)); - BEAST_EXPECT(env.balance(gw, minter["XAU"]) == gwXAU(-30)); - BEAST_EXPECT( - env.balance(gw, secondarySeller["XAU"]) == gwXAU(-970)); - BEAST_EXPECT(env.balance(gw, buyer["XAU"]) == gwXAU(20)); - } + expectInitialState(); } { // There is a transfer fee on the NFT and buyer has exact @@ -5815,26 +5505,12 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite // now we can do a secondary sale auto const offerID = createBuyOffer(buyer, secondarySeller, nftID, gwXAU(1000)); - auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] - ? static_cast(tecINSUFFICIENT_FUNDS) - : static_cast(tesSUCCESS); + TER const sellTER = tecINSUFFICIENT_FUNDS; env(token::acceptBuyOffer(secondarySeller, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixNonFungibleTokensV1_2]) - expectInitialState(); - else - { - BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(30)); - BEAST_EXPECT( - env.balance(secondarySeller, gwXAU) == gwXAU(970)); - BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(-20)); - BEAST_EXPECT(env.balance(gw, minter["XAU"]) == gwXAU(-30)); - BEAST_EXPECT( - env.balance(gw, secondarySeller["XAU"]) == gwXAU(-970)); - BEAST_EXPECT(env.balance(gw, buyer["XAU"]) == gwXAU(20)); - } + expectInitialState(); } { // There is a transfer fee on the NFT and buyer has enough @@ -5988,8 +5664,6 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite // the NFToken being bought and returned to the original owner and // the broker pocketing the profit. // - // This unit test verifies that the fixNonFungibleTokensV1_2 amendment - // fixes that bug. testcase("Brokered sale to self"); using namespace test::jtx; @@ -6055,40 +5729,24 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite BEAST_EXPECT(nftCount(env, bob) == 1); auto const bobsPriorBalance = env.balance(bob); auto const brokersPriorBalance = env.balance(broker); - TER expectTer = features[fixNonFungibleTokensV1_2] - ? TER(tecCANT_ACCEPT_OWN_NFTOKEN_OFFER) - : TER(tesSUCCESS); env(token::brokerOffers(broker, bobBuyOfferIndex, bobSellOfferIndex), token::brokerFee(XRP(1)), - ter(expectTer)); + ter(tecCANT_ACCEPT_OWN_NFTOKEN_OFFER)); env.close(); - if (expectTer == tesSUCCESS) - { - // bob should still have the NFT from alice, but be XRP(1) poorer. - // broker should be almost XRP(1) richer because they also paid a - // transaction fee. - BEAST_EXPECT(nftCount(env, bob) == 1); - BEAST_EXPECT(env.balance(bob) == bobsPriorBalance - XRP(1)); - BEAST_EXPECT( - env.balance(broker) == brokersPriorBalance + XRP(1) - baseFee); - } - else - { - // A tec result was returned, so no state should change other - // than the broker burning their transaction fee. - BEAST_EXPECT(nftCount(env, bob) == 1); - BEAST_EXPECT(env.balance(bob) == bobsPriorBalance); - BEAST_EXPECT(env.balance(broker) == brokersPriorBalance - baseFee); - } + // A tec result was returned, so no state should change other + // than the broker burning their transaction fee. + BEAST_EXPECT(nftCount(env, bob) == 1); + BEAST_EXPECT(env.balance(bob) == bobsPriorBalance); + BEAST_EXPECT(env.balance(broker) == brokersPriorBalance - baseFee); } void - testFixNFTokenRemint(FeatureBitset features) + testNFTokenRemint(FeatureBitset features) { using namespace test::jtx; - testcase("fixNFTokenRemint"); + testcase("NFTokenRemint"); // Returns the current ledger sequence auto openLedgerSeq = [](Env& env) { return env.current()->seq(); }; @@ -6110,7 +5768,6 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite // Close the ledger until the ledger sequence is no longer // within . - // This is enforced by the fixNFTokenRemint amendment. auto incLgrSeqForFixNftRemint = [&](Env& env, Account const& acct) { int delta = 0; auto const deletableLgrSeq = @@ -6178,12 +5835,8 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite env(token::burn(alice, remintNFTokenID)); env.close(); - if (features[fixNFTokenRemint]) - // Check that two NFTs don't have the same ID - BEAST_EXPECT(remintNFTokenID != prevNFTokenID); - else - // Check that two NFTs have the same ID - BEAST_EXPECT(remintNFTokenID == prevNFTokenID); + // Check that two NFTs don't have the same ID + BEAST_EXPECT(remintNFTokenID != prevNFTokenID); } // Test if the issuer account can be deleted after an authorized @@ -6230,96 +5883,57 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite auto const acctDelFee{drops(env.current()->fees().increment)}; - if (!features[fixNFTokenRemint]) - { - // alice's account can be successfully deleted. - env(acctdelete(alice, becky), fee(acctDelFee)); - env.close(); - BEAST_EXPECT(!env.current()->exists(aliceAcctKey)); + // alice tries to delete her account, but is unsuccessful. + // Due to authorized minting, alice's account sequence does not + // advance while minter mints NFTokens for her. + // The new account deletion retriction enabled by this amendment will enforce + // alice to wait for more ledgers to close before she can + // delete her account, to prevent duplicate NFTokenIDs + env(acctdelete(alice, becky), fee(acctDelFee), ter(tecTOO_SOON)); + env.close(); - // Fund alice to re-create her account - env.fund(XRP(10000), alice); - env.close(); + // alice's account is still present + BEAST_EXPECT(env.current()->exists(aliceAcctKey)); - // alice's account now exists and has minted 0 NFTokens - BEAST_EXPECT(env.closed()->exists(aliceAcctKey)); - BEAST_EXPECT(env.current()->exists(aliceAcctKey)); - BEAST_EXPECT((*env.le(alice))[sfMintedNFTokens] == 0); + // Close more ledgers until it is no longer within + // + // to be able to delete alice's account + incLgrSeqForFixNftRemint(env, alice); - // alice mints a NFT with same params as the first one before - // the account delete. - uint256 const remintNFTokenID = - token::getNextID(env, alice, 0u); - env(token::mint(alice)); - env.close(); + // alice's account is deleted + env(acctdelete(alice, becky), fee(acctDelFee)); + env.close(); - // burn the NFT to make sure alice owns remintNFTokenID - env(token::burn(alice, remintNFTokenID)); - env.close(); + // alice's account root is gone from the most recently + // closed ledger and the current ledger. + BEAST_EXPECT(!env.closed()->exists(aliceAcctKey)); + BEAST_EXPECT(!env.current()->exists(aliceAcctKey)); - // The new NFT minted has the same ID as one of the NFTs - // authorized minter minted for alice - BEAST_EXPECT( - std::find(nftIDs.begin(), nftIDs.end(), remintNFTokenID) != - nftIDs.end()); - } - else if (features[fixNFTokenRemint]) - { - // alice tries to delete her account, but is unsuccessful. - // Due to authorized minting, alice's account sequence does not - // advance while minter mints NFTokens for her. - // The new account deletion retriction enabled by this amendment will enforce - // alice to wait for more ledgers to close before she can - // delete her account, to prevent duplicate NFTokenIDs - env(acctdelete(alice, becky), - fee(acctDelFee), - ter(tecTOO_SOON)); - env.close(); + // Fund alice to re-create her account + env.fund(XRP(10000), alice); + env.close(); - // alice's account is still present - BEAST_EXPECT(env.current()->exists(aliceAcctKey)); + // alice's account now exists and has minted 0 NFTokens + BEAST_EXPECT(env.closed()->exists(aliceAcctKey)); + BEAST_EXPECT(env.current()->exists(aliceAcctKey)); + BEAST_EXPECT((*env.le(alice))[sfMintedNFTokens] == 0); - // Close more ledgers until it is no longer within - // - // to be able to delete alice's account - incLgrSeqForFixNftRemint(env, alice); + // alice mints a NFT with same params as the first one before + // the account delete. + uint256 const remintNFTokenID = token::getNextID(env, alice, 0u); + env(token::mint(alice)); + env.close(); - // alice's account is deleted - env(acctdelete(alice, becky), fee(acctDelFee)); - env.close(); + // burn the NFT to make sure alice owns remintNFTokenID + env(token::burn(alice, remintNFTokenID)); + env.close(); - // alice's account root is gone from the most recently - // closed ledger and the current ledger. - BEAST_EXPECT(!env.closed()->exists(aliceAcctKey)); - BEAST_EXPECT(!env.current()->exists(aliceAcctKey)); - - // Fund alice to re-create her account - env.fund(XRP(10000), alice); - env.close(); - - // alice's account now exists and has minted 0 NFTokens - BEAST_EXPECT(env.closed()->exists(aliceAcctKey)); - BEAST_EXPECT(env.current()->exists(aliceAcctKey)); - BEAST_EXPECT((*env.le(alice))[sfMintedNFTokens] == 0); - - // alice mints a NFT with same params as the first one before - // the account delete. - uint256 const remintNFTokenID = - token::getNextID(env, alice, 0u); - env(token::mint(alice)); - env.close(); - - // burn the NFT to make sure alice owns remintNFTokenID - env(token::burn(alice, remintNFTokenID)); - env.close(); - - // The new NFT minted will not have the same ID - // as any of the NFTs authorized minter minted - BEAST_EXPECT( - std::find(nftIDs.begin(), nftIDs.end(), remintNFTokenID) == - nftIDs.end()); - } + // The new NFT minted will not have the same ID + // as any of the NFTs authorized minter minted + BEAST_EXPECT( + std::find(nftIDs.begin(), nftIDs.end(), remintNFTokenID) == + nftIDs.end()); } // When an account mints and burns a batch of NFTokens using tickets, @@ -6373,169 +5987,13 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite auto const acctDelFee{drops(env.current()->fees().increment)}; - if (!features[fixNFTokenRemint]) - { - // alice tries to delete her account, and is successful. - env(acctdelete(alice, becky), fee(acctDelFee)); - env.close(); - - // alice's account root is gone from the most recently - // closed ledger and the current ledger. - BEAST_EXPECT(!env.closed()->exists(aliceAcctKey)); - BEAST_EXPECT(!env.current()->exists(aliceAcctKey)); - - // Fund alice to re-create her account - env.fund(XRP(10000), alice); - env.close(); - - // alice's account now exists and has minted 0 NFTokens - BEAST_EXPECT(env.closed()->exists(aliceAcctKey)); - BEAST_EXPECT(env.current()->exists(aliceAcctKey)); - BEAST_EXPECT((*env.le(alice))[sfMintedNFTokens] == 0); - - // alice mints a NFT with same params as the first one before - // the account delete. - uint256 const remintNFTokenID = - token::getNextID(env, alice, 0u); - env(token::mint(alice)); - env.close(); - - // burn the NFT to make sure alice owns remintNFTokenID - env(token::burn(alice, remintNFTokenID)); - env.close(); - - // The new NFT minted will have the same ID - // as one of NFTs minted using tickets - BEAST_EXPECT( - std::find(nftIDs.begin(), nftIDs.end(), remintNFTokenID) != - nftIDs.end()); - } - else if (features[fixNFTokenRemint]) - { - // alice tries to delete her account, but is unsuccessful. - // Due to authorized minting, alice's account sequence does not - // advance while minter mints NFTokens for her using tickets. - // The new account deletion retriction enabled by this amendment will enforce - // alice to wait for more ledgers to close before she can - // delete her account, to prevent duplicate NFTokenIDs - env(acctdelete(alice, becky), - fee(acctDelFee), - ter(tecTOO_SOON)); - env.close(); - - // alice's account is still present - BEAST_EXPECT(env.current()->exists(aliceAcctKey)); - - // Close more ledgers until it is no longer within - // - // to be able to delete alice's account - incLgrSeqForFixNftRemint(env, alice); - - // alice's account is deleted - env(acctdelete(alice, becky), fee(acctDelFee)); - env.close(); - - // alice's account root is gone from the most recently - // closed ledger and the current ledger. - BEAST_EXPECT(!env.closed()->exists(aliceAcctKey)); - BEAST_EXPECT(!env.current()->exists(aliceAcctKey)); - - // Fund alice to re-create her account - env.fund(XRP(10000), alice); - env.close(); - - // alice's account now exists and has minted 0 NFTokens - BEAST_EXPECT(env.closed()->exists(aliceAcctKey)); - BEAST_EXPECT(env.current()->exists(aliceAcctKey)); - BEAST_EXPECT((*env.le(alice))[sfMintedNFTokens] == 0); - - // alice mints a NFT with same params as the first one before - // the account delete. - uint256 const remintNFTokenID = - token::getNextID(env, alice, 0u); - env(token::mint(alice)); - env.close(); - - // burn the NFT to make sure alice owns remintNFTokenID - env(token::burn(alice, remintNFTokenID)); - env.close(); - - // The new NFT minted will not have the same ID - // as any of the NFTs authorized minter minted using tickets - BEAST_EXPECT( - std::find(nftIDs.begin(), nftIDs.end(), remintNFTokenID) == - nftIDs.end()); - } - } - // If fixNFTokenRemint is enabled, - // when an authorized minter mints and burns a batch of NFTokens using - // tickets, issuer's account needs to wait a longer time before it can - // deleted. - // After the issuer's account is re-created and mints a NFT, it should - // not have the same NFTokenID as the ones authorized minter minted. - if (features[fixNFTokenRemint]) - { - Env env{*this, features}; - Account const alice("alice"); - Account const becky("becky"); - Account const minter{"minter"}; - - env.fund(XRP(10000), alice, becky, minter); - env.close(); - - // alice sets minter as her authorized minter - env(token::setMinter(alice, minter)); - env.close(); - - // minter creates 100 tickets - std::uint32_t minterTicketSeq{env.seq(minter) + 1}; - env(ticket::create(minter, 100)); - env.close(); - - BEAST_EXPECT(ticketCount(env, minter) == 100); - BEAST_EXPECT(ownerCount(env, minter) == 100); - - // minter mints 50 NFTs for alice using tickets - std::vector nftIDs; - nftIDs.reserve(50); - for (int i = 0; i < 50; i++) - { - uint256 const nftokenID = token::getNextID(env, alice, 0u); - nftIDs.push_back(nftokenID); - env(token::mint(minter), - token::issuer(alice), - ticket::use(minterTicketSeq++)); - } - env.close(); - - // minter burns 50 NFTs using tickets - for (auto const nftokenID : nftIDs) - { - env(token::burn(minter, nftokenID), - ticket::use(minterTicketSeq++)); - } - env.close(); - - BEAST_EXPECT(ticketCount(env, minter) == 0); - - // Increment ledger sequence to the number that is - // enforced by the featureDeletableAccounts amendment - incLgrSeqForAcctDel(env, alice); - - // Verify that alice's account root is present. - Keylet const aliceAcctKey{keylet::account(alice.id())}; - BEAST_EXPECT(env.closed()->exists(aliceAcctKey)); - BEAST_EXPECT(env.current()->exists(aliceAcctKey)); - // alice tries to delete her account, but is unsuccessful. // Due to authorized minting, alice's account sequence does not // advance while minter mints NFTokens for her using tickets. // The new account deletion retriction enabled by this amendment will enforce - // alice to wait for more ledgers to close before she can delete her - // account, to prevent duplicate NFTokenIDs - auto const acctDelFee{drops(env.current()->fees().increment)}; + // alice to wait for more ledgers to close before she can + // delete her account, to prevent duplicate NFTokenIDs env(acctdelete(alice, becky), fee(acctDelFee), ter(tecTOO_SOON)); env.close(); @@ -6565,8 +6023,8 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite BEAST_EXPECT(env.current()->exists(aliceAcctKey)); BEAST_EXPECT((*env.le(alice))[sfMintedNFTokens] == 0); - // The new NFT minted will not have the same ID - // as any of the NFTs authorized minter minted using tickets + // alice mints a NFT with same params as the first one before + // the account delete. uint256 const remintNFTokenID = token::getNextID(env, alice, 0u); env(token::mint(alice)); env.close(); @@ -6576,11 +6034,119 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite env.close(); // The new NFT minted will not have the same ID - // as one of NFTs authorized minter minted using tickets + // as any of the NFTs authorized minter minted using tickets BEAST_EXPECT( std::find(nftIDs.begin(), nftIDs.end(), remintNFTokenID) == nftIDs.end()); } + // When an authorized minter mints and burns a batch of NFTokens using + // tickets, issuer's account needs to wait a longer time before it can + // be deleted. + // After the issuer's account is re-created and mints a NFT, it should + // not have the same NFTokenID as the ones authorized minter minted. + Env env{*this, features}; + Account const alice("alice"); + Account const becky("becky"); + Account const minter{"minter"}; + + env.fund(XRP(10000), alice, becky, minter); + env.close(); + + // alice sets minter as her authorized minter + env(token::setMinter(alice, minter)); + env.close(); + + // minter creates 100 tickets + std::uint32_t minterTicketSeq{env.seq(minter) + 1}; + env(ticket::create(minter, 100)); + env.close(); + + BEAST_EXPECT(ticketCount(env, minter) == 100); + BEAST_EXPECT(ownerCount(env, minter) == 100); + + // minter mints 50 NFTs for alice using tickets + std::vector nftIDs; + nftIDs.reserve(50); + for (int i = 0; i < 50; i++) + { + uint256 const nftokenID = token::getNextID(env, alice, 0u); + nftIDs.push_back(nftokenID); + env(token::mint(minter), + token::issuer(alice), + ticket::use(minterTicketSeq++)); + } + env.close(); + + // minter burns 50 NFTs using tickets + for (auto const nftokenID : nftIDs) + { + env(token::burn(minter, nftokenID), ticket::use(minterTicketSeq++)); + } + env.close(); + + BEAST_EXPECT(ticketCount(env, minter) == 0); + + // Increment ledger sequence to the number that is + // enforced by the featureDeletableAccounts amendment + incLgrSeqForAcctDel(env, alice); + + // Verify that alice's account root is present. + Keylet const aliceAcctKey{keylet::account(alice.id())}; + BEAST_EXPECT(env.closed()->exists(aliceAcctKey)); + BEAST_EXPECT(env.current()->exists(aliceAcctKey)); + + // alice tries to delete her account, but is unsuccessful. + // Due to authorized minting, alice's account sequence does not + // advance while minter mints NFTokens for her using tickets. + // The new account deletion retriction enabled by this amendment will enforce + // alice to wait for more ledgers to close before she can delete her + // account, to prevent duplicate NFTokenIDs + auto const acctDelFee{drops(env.current()->fees().increment)}; + env(acctdelete(alice, becky), fee(acctDelFee), ter(tecTOO_SOON)); + env.close(); + + // alice's account is still present + BEAST_EXPECT(env.current()->exists(aliceAcctKey)); + + // Close more ledgers until it is no longer within + // + // to be able to delete alice's account + incLgrSeqForFixNftRemint(env, alice); + + // alice's account is deleted + env(acctdelete(alice, becky), fee(acctDelFee)); + env.close(); + + // alice's account root is gone from the most recently + // closed ledger and the current ledger. + BEAST_EXPECT(!env.closed()->exists(aliceAcctKey)); + BEAST_EXPECT(!env.current()->exists(aliceAcctKey)); + + // Fund alice to re-create her account + env.fund(XRP(10000), alice); + env.close(); + + // alice's account now exists and has minted 0 NFTokens + BEAST_EXPECT(env.closed()->exists(aliceAcctKey)); + BEAST_EXPECT(env.current()->exists(aliceAcctKey)); + BEAST_EXPECT((*env.le(alice))[sfMintedNFTokens] == 0); + + // The new NFT minted will not have the same ID + // as any of the NFTs authorized minter minted using tickets + uint256 const remintNFTokenID = token::getNextID(env, alice, 0u); + env(token::mint(alice)); + env.close(); + + // burn the NFT to make sure alice owns remintNFTokenID + env(token::burn(alice, remintNFTokenID)); + env.close(); + + // The new NFT minted will not have the same ID + // as one of NFTs authorized minter minted using tickets + BEAST_EXPECT( + std::find(nftIDs.begin(), nftIDs.end(), remintNFTokenID) == + nftIDs.end()); } void @@ -6835,27 +6401,16 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite BEAST_EXPECT(ownerCount(env, issuer) == 0); } - // Test sell offers with a destination with and without - // fixNFTokenNegOffer. - for (auto const& tweakedFeatures : - {features - fixNFTokenNegOffer - featureNonFungibleTokensV1_1, - features | fixNFTokenNegOffer}) - { - Env env{*this, tweakedFeatures}; - Account const alice("alice"); + Env env{*this, features}; + Account const alice("alice"); - env.fund(XRP(1000000), alice); + env.fund(XRP(1000000), alice); - TER const offerCreateTER = tweakedFeatures[fixNFTokenNegOffer] - ? static_cast(temBAD_AMOUNT) - : static_cast(tesSUCCESS); + TER const offerCreateTER = temBAD_AMOUNT; - // Make offers with negative amounts for the NFTs - env(token::mint(alice), - token::amount(XRP(-2)), - ter(offerCreateTER)); - env.close(); - } + // Make offers with negative amounts for the NFTs + env(token::mint(alice), token::amount(XRP(-2)), ter(offerCreateTER)); + env.close(); } void @@ -8029,6 +7584,9 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite } } +protected: + FeatureBitset const allFeatures{test::jtx::testable_amendments()}; + void testWithFeats(FeatureBitset features) { @@ -8057,10 +7615,10 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite testNFTokenWithTickets(features); testNFTokenDeleteAccount(features); testNftXxxOffers(features); - testFixNFTokenNegOffer(features); + testNFTokenNegOffer(features); testIOUWithTransferFee(features); testBrokeredSaleToSelf(features); - testFixNFTokenRemint(features); + testNFTokenRemint(features); testFeatMintWithOffer(features); testTxJsonMetaFields(features); testFixNFTokenBuyerReserve(features); @@ -8070,40 +7628,12 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite } public: - void - run(std::uint32_t instance, bool last = false) - { - using namespace test::jtx; - static FeatureBitset const all{testable_amendments()}; - static FeatureBitset const fixNFTDir{fixNFTokenDirV1}; - - static std::array const feats{ - all - fixNFTDir - fixNonFungibleTokensV1_2 - fixNFTokenRemint - - fixNFTokenReserve - featureNFTokenMintOffer - featureDynamicNFT, - all - disallowIncoming - fixNonFungibleTokensV1_2 - - fixNFTokenRemint - fixNFTokenReserve - featureNFTokenMintOffer - - featureDynamicNFT, - all - fixNonFungibleTokensV1_2 - fixNFTokenRemint - - fixNFTokenReserve - featureNFTokenMintOffer - featureDynamicNFT, - all - fixNFTokenRemint - fixNFTokenReserve - - featureNFTokenMintOffer - featureDynamicNFT, - all - fixNFTokenReserve - featureNFTokenMintOffer - - featureDynamicNFT, - all - featureNFTokenMintOffer - featureDynamicNFT, - all - featureDynamicNFT, - all}; - - if (BEAST_EXPECT(instance < feats.size())) - { - testWithFeats(feats[instance]); - } - BEAST_EXPECT(!last || instance == feats.size() - 1); - } - void run() override { - run(0); + testWithFeats( + allFeatures - fixNFTokenReserve - featureNFTokenMintOffer - + featureDynamicNFT); } }; @@ -8112,34 +7642,9 @@ class NFTokenDisallowIncoming_test : public NFTokenBaseUtil_test void run() override { - NFTokenBaseUtil_test::run(1); - } -}; - -class NFTokenWOfixV1_test : public NFTokenBaseUtil_test -{ - void - run() override - { - NFTokenBaseUtil_test::run(2); - } -}; - -class NFTokenWOTokenRemint_test : public NFTokenBaseUtil_test -{ - void - run() override - { - NFTokenBaseUtil_test::run(3); - } -}; - -class NFTokenWOTokenReserve_test : public NFTokenBaseUtil_test -{ - void - run() override - { - NFTokenBaseUtil_test::run(4); + testWithFeats( + allFeatures - featureDisallowIncoming - fixNFTokenReserve - + featureNFTokenMintOffer - featureDynamicNFT); } }; @@ -8148,7 +7653,8 @@ class NFTokenWOMintOffer_test : public NFTokenBaseUtil_test void run() override { - NFTokenBaseUtil_test::run(5); + testWithFeats( + allFeatures - featureNFTokenMintOffer - featureDynamicNFT); } }; @@ -8157,7 +7663,7 @@ class NFTokenWOModify_test : public NFTokenBaseUtil_test void run() override { - NFTokenBaseUtil_test::run(6); + testWithFeats(allFeatures - featureDynamicNFT); } }; @@ -8166,15 +7672,12 @@ class NFTokenAllFeatures_test : public NFTokenBaseUtil_test void run() override { - NFTokenBaseUtil_test::run(7, true); + testWithFeats(allFeatures); } }; BEAST_DEFINE_TESTSUITE_PRIO(NFTokenBaseUtil, app, ripple, 2); BEAST_DEFINE_TESTSUITE_PRIO(NFTokenDisallowIncoming, app, ripple, 2); -BEAST_DEFINE_TESTSUITE_PRIO(NFTokenWOfixV1, app, ripple, 2); -BEAST_DEFINE_TESTSUITE_PRIO(NFTokenWOTokenRemint, app, ripple, 2); -BEAST_DEFINE_TESTSUITE_PRIO(NFTokenWOTokenReserve, app, ripple, 2); BEAST_DEFINE_TESTSUITE_PRIO(NFTokenWOMintOffer, app, ripple, 2); BEAST_DEFINE_TESTSUITE_PRIO(NFTokenWOModify, app, ripple, 2); BEAST_DEFINE_TESTSUITE_PRIO(NFTokenAllFeatures, app, ripple, 2); diff --git a/src/test/jtx/impl/token.cpp b/src/test/jtx/impl/token.cpp index bea974a016..3bc4e9bd02 100644 --- a/src/test/jtx/impl/token.cpp +++ b/src/test/jtx/impl/token.cpp @@ -87,14 +87,10 @@ getID( std::uint16_t flags, std::uint16_t xferFee) { - if (env.current()->rules().enabled(fixNFTokenRemint)) - { - // If fixNFTokenRemint is enabled, we must add issuer's - // FirstNFTokenSequence to offset the starting NFT sequence number. - nftSeq += env.le(issuer) - ->at(~sfFirstNFTokenSequence) - .value_or(env.seq(issuer)); - } + // We must add issuer's FirstNFTokenSequence to offset the starting NFT + // sequence number. + nftSeq += + env.le(issuer)->at(~sfFirstNFTokenSequence).value_or(env.seq(issuer)); return ripple::NFTokenMint::createNFTokenID( flags, xferFee, issuer, nft::toTaxon(nfTokenTaxon), nftSeq); } diff --git a/src/test/rpc/Feature_test.cpp b/src/test/rpc/Feature_test.cpp index ea847a5a83..8e405a9a3c 100644 --- a/src/test/rpc/Feature_test.cpp +++ b/src/test/rpc/Feature_test.cpp @@ -547,7 +547,7 @@ class Feature_test : public beast::unit_test::suite using namespace test::jtx; Env env{*this}; - constexpr char const* featureName = "NonFungibleTokensV1"; + constexpr char const* featureName = "CryptoConditionsSuite"; auto jrr = env.rpc("feature", featureName)[jss::result]; if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status")) diff --git a/src/xrpld/app/tx/detail/DeleteAccount.cpp b/src/xrpld/app/tx/detail/DeleteAccount.cpp index 805c023c04..e2f890f9fc 100644 --- a/src/xrpld/app/tx/detail/DeleteAccount.cpp +++ b/src/xrpld/app/tx/detail/DeleteAccount.cpp @@ -265,24 +265,20 @@ DeleteAccount::preclaim(PreclaimContext const& ctx) if (!sleAccount) return terNO_ACCOUNT; - if (ctx.view.rules().enabled(featureNonFungibleTokensV1)) - { - // If an issuer has any issued NFTs resident in the ledger then it - // cannot be deleted. - if ((*sleAccount)[~sfMintedNFTokens] != - (*sleAccount)[~sfBurnedNFTokens]) - return tecHAS_OBLIGATIONS; + // If an issuer has any issued NFTs resident in the ledger then it + // cannot be deleted. + if ((*sleAccount)[~sfMintedNFTokens] != (*sleAccount)[~sfBurnedNFTokens]) + return tecHAS_OBLIGATIONS; - // If the account owns any NFTs it cannot be deleted. - Keylet const first = keylet::nftpage_min(account); - Keylet const last = keylet::nftpage_max(account); + // If the account owns any NFTs it cannot be deleted. + Keylet const first = keylet::nftpage_min(account); + Keylet const last = keylet::nftpage_max(account); - auto const cp = ctx.view.read(Keylet( - ltNFTOKEN_PAGE, - ctx.view.succ(first.key, last.key.next()).value_or(last.key))); - if (cp) - return tecHAS_OBLIGATIONS; - } + auto const cp = ctx.view.read(Keylet( + ltNFTOKEN_PAGE, + ctx.view.succ(first.key, last.key.next()).value_or(last.key))); + if (cp) + return tecHAS_OBLIGATIONS; // We don't allow an account to be deleted if its sequence number // is within 256 of the current ledger. This prevents replay of old @@ -294,8 +290,8 @@ DeleteAccount::preclaim(PreclaimContext const& ctx) if ((*sleAccount)[sfSequence] + seqDelta > ctx.view.seq()) return tecTOO_SOON; - // When fixNFTokenRemint is enabled, we don't allow an account to be - // deleted if is within 256 of the + // We don't allow an account to be deleted if + // is within 256 of the // current ledger. This is to prevent having duplicate NFTokenIDs after // account re-creation. // @@ -305,10 +301,9 @@ DeleteAccount::preclaim(PreclaimContext const& ctx) // their account and mints a NFToken, it is possible that the // NFTokenSequence of this NFToken is the same as the one that the // authorized minter minted in a previous ledger. - if (ctx.view.rules().enabled(fixNFTokenRemint) && - ((*sleAccount)[~sfFirstNFTokenSequence].value_or(0) + - (*sleAccount)[~sfMintedNFTokens].value_or(0) + seqDelta > - ctx.view.seq())) + if ((*sleAccount)[~sfFirstNFTokenSequence].value_or(0) + + (*sleAccount)[~sfMintedNFTokens].value_or(0) + seqDelta > + ctx.view.seq()) return tecTOO_SOON; // Verify that the account does not own any objects that would prevent diff --git a/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp b/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp index 23874ee3e0..874aaba654 100644 --- a/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp +++ b/src/xrpld/app/tx/detail/NFTokenAcceptOffer.cpp @@ -75,10 +75,7 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx) if (hasExpired(ctx.view, (*offerSLE)[~sfExpiration])) return {nullptr, tecEXPIRED}; - // The initial implementation had a bug that allowed a negative - // amount. The fixNFTokenNegOffer amendment fixes that. - if ((*offerSLE)[sfAmount].negative() && - ctx.view.rules().enabled(fixNFTokenNegOffer)) + if ((*offerSLE)[sfAmount].negative()) return {nullptr, temBAD_OFFER}; return {std::move(offerSLE), tesSUCCESS}; @@ -106,8 +103,7 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx) // The two offers may not form a loop. A broker may not sell the // token to the current owner of the token. - if (ctx.view.rules().enabled(fixNonFungibleTokensV1_2) && - ((*bo)[sfOwner] == (*so)[sfOwner])) + if (((*bo)[sfOwner] == (*so)[sfOwner])) return tecCANT_ACCEPT_OWN_NFTOKEN_OFFER; // Ensure that the buyer is willing to pay at least as much as the @@ -115,32 +111,20 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx) if ((*so)[sfAmount] > (*bo)[sfAmount]) return tecINSUFFICIENT_PAYMENT; - // If the buyer specified a destination - if (auto const dest = bo->at(~sfDestination)) + // The destination must be whoever is submitting the tx if the buyer + // specified it + if (auto const dest = bo->at(~sfDestination); + dest && *dest != ctx.tx[sfAccount]) { - // Before this fix the destination could be either the seller or - // a broker. After, it must be whoever is submitting the tx. - if (ctx.view.rules().enabled(fixNonFungibleTokensV1_2)) - { - if (*dest != ctx.tx[sfAccount]) - return tecNO_PERMISSION; - } - else if (*dest != so->at(sfOwner) && *dest != ctx.tx[sfAccount]) - return tecNFTOKEN_BUY_SELL_MISMATCH; + return tecNO_PERMISSION; } - // If the seller specified a destination - if (auto const dest = so->at(~sfDestination)) + // The destination must be whoever is submitting the tx if the seller + // specified it + if (auto const dest = so->at(~sfDestination); + dest && *dest != ctx.tx[sfAccount]) { - // Before this fix the destination could be either the seller or - // a broker. After, it must be whoever is submitting the tx. - if (ctx.view.rules().enabled(fixNonFungibleTokensV1_2)) - { - if (*dest != ctx.tx[sfAccount]) - return tecNO_PERMISSION; - } - else if (*dest != bo->at(sfOwner) && *dest != ctx.tx[sfAccount]) - return tecNFTOKEN_BUY_SELL_MISMATCH; + return tecNO_PERMISSION; } // The broker can specify an amount that represents their cut; if they @@ -210,21 +194,10 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx) // After this amendment, we allow an IOU issuer to buy an NFT with their // own currency auto const needed = bo->at(sfAmount); - if (ctx.view.rules().enabled(fixNonFungibleTokensV1_2)) - { - if (accountFunds( - ctx.view, (*bo)[sfOwner], needed, fhZERO_IF_FROZEN, ctx.j) < - needed) - return tecINSUFFICIENT_FUNDS; - } - else if ( - accountHolds( - ctx.view, - (*bo)[sfOwner], - needed.getCurrency(), - needed.getIssuer(), - fhZERO_IF_FROZEN, - ctx.j) < needed) + + if (accountFunds( + ctx.view, (*bo)[sfOwner], needed, fhZERO_IF_FROZEN, ctx.j) < + needed) return tecINSUFFICIENT_FUNDS; // Check that the account accepting the buy offer (he's selling the NFT) @@ -285,18 +258,7 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx) // The account offering to buy must have funds: auto const needed = so->at(sfAmount); - if (!ctx.view.rules().enabled(fixNonFungibleTokensV1_2)) - { - if (accountHolds( - ctx.view, - ctx.tx[sfAccount], - needed.getCurrency(), - needed.getIssuer(), - fhZERO_IF_FROZEN, - ctx.j) < needed) - return tecINSUFFICIENT_FUNDS; - } - else if (!bo) + if (!bo) { // After this amendment, we allow buyers to buy with their own // issued currency. @@ -403,13 +365,11 @@ NFTokenAcceptOffer::pay( auto const result = accountSend(view(), from, to, amount, j_); - // After this amendment, if any payment would cause a non-IOU-issuer to - // have a negative balance, or an IOU-issuer to have a positive balance in - // their own currency, we know that something went wrong. This was - // originally found in the context of IOU transfer fees. Since there are - // several payouts in this tx, just confirm that the end state is OK. - if (!view().rules().enabled(fixNonFungibleTokensV1_2)) - return result; + // If any payment causes a non-IOU-issuer to have a negative balance, + // or an IOU-issuer to have a positive balance in their own currency, + // we know that something went wrong. This was originally found in the + // context of IOU transfer fees. Since there are several payouts in this tx, + // just confirm that the end state is OK. if (result != tesSUCCESS) return result; if (accountFunds(view(), from, amount, fhZERO_IF_FROZEN, j_).signum() < 0) diff --git a/src/xrpld/app/tx/detail/NFTokenBurn.cpp b/src/xrpld/app/tx/detail/NFTokenBurn.cpp index cb1b564402..aafb3ee4b3 100644 --- a/src/xrpld/app/tx/detail/NFTokenBurn.cpp +++ b/src/xrpld/app/tx/detail/NFTokenBurn.cpp @@ -64,13 +64,6 @@ NFTokenBurn::preclaim(PreclaimContext const& ctx) } } - if (!ctx.view.rules().enabled(fixNonFungibleTokensV1_2)) - { - // If there are too many offers, then burning the token would produce - // too much metadata. Disallow burning a token with too many offers. - return nft::notTooManyOffers(ctx.view, ctx.tx[sfNFTokenID]); - } - return tesSUCCESS; } @@ -96,37 +89,21 @@ NFTokenBurn::doApply() view().update(issuer); } - if (ctx_.view().rules().enabled(fixNonFungibleTokensV1_2)) - { - // Delete up to 500 offers in total. - // Because the number of sell offers is likely to be less than - // the number of buy offers, we prioritize the deletion of sell - // offers in order to clean up sell offer directory - std::size_t const deletedSellOffers = nft::removeTokenOffersWithLimit( - view(), - keylet::nft_sells(ctx_.tx[sfNFTokenID]), - maxDeletableTokenOfferEntries); + // Delete up to 500 offers in total. + // Because the number of sell offers is likely to be less than + // the number of buy offers, we prioritize the deletion of sell + // offers in order to clean up sell offer directory + std::size_t const deletedSellOffers = nft::removeTokenOffersWithLimit( + view(), + keylet::nft_sells(ctx_.tx[sfNFTokenID]), + maxDeletableTokenOfferEntries); - if (maxDeletableTokenOfferEntries > deletedSellOffers) - { - nft::removeTokenOffersWithLimit( - view(), - keylet::nft_buys(ctx_.tx[sfNFTokenID]), - maxDeletableTokenOfferEntries - deletedSellOffers); - } - } - else + if (maxDeletableTokenOfferEntries > deletedSellOffers) { - // Deletion of all offers. - nft::removeTokenOffersWithLimit( - view(), - keylet::nft_sells(ctx_.tx[sfNFTokenID]), - std::numeric_limits::max()); - nft::removeTokenOffersWithLimit( view(), keylet::nft_buys(ctx_.tx[sfNFTokenID]), - std::numeric_limits::max()); + maxDeletableTokenOfferEntries - deletedSellOffers); } return tesSUCCESS; diff --git a/src/xrpld/app/tx/detail/NFTokenMint.cpp b/src/xrpld/app/tx/detail/NFTokenMint.cpp index dd82443fee..da615a5dad 100644 --- a/src/xrpld/app/tx/detail/NFTokenMint.cpp +++ b/src/xrpld/app/tx/detail/NFTokenMint.cpp @@ -230,24 +230,6 @@ NFTokenMint::doApply() // Should not happen. Checked in preclaim. return Unexpected(tecNO_ISSUER); - if (!ctx_.view().rules().enabled(fixNFTokenRemint)) - { - // Get the unique sequence number for this token: - std::uint32_t const tokenSeq = - (*root)[~sfMintedNFTokens].value_or(0); - { - std::uint32_t const nextTokenSeq = tokenSeq + 1; - if (nextTokenSeq < tokenSeq) - return Unexpected(tecMAX_SEQUENCE_REACHED); - - (*root)[sfMintedNFTokens] = nextTokenSeq; - } - ctx_.view().update(root); - return tokenSeq; - } - - // With fixNFTokenRemint amendment enabled: - // // If the issuer hasn't minted an NFToken before we must add a // FirstNFTokenSequence field to the issuer's AccountRoot. The // value of the FirstNFTokenSequence must equal the issuer's diff --git a/src/xrpld/app/tx/detail/NFTokenModify.cpp b/src/xrpld/app/tx/detail/NFTokenModify.cpp index 6ae095411b..b3447f4558 100644 --- a/src/xrpld/app/tx/detail/NFTokenModify.cpp +++ b/src/xrpld/app/tx/detail/NFTokenModify.cpp @@ -25,12 +25,6 @@ namespace ripple { -bool -NFTokenModify::checkExtraFeatures(PreflightContext const& ctx) -{ - return ctx.rules.enabled(featureNonFungibleTokensV1_1); -} - NotTEC NFTokenModify::preflight(PreflightContext const& ctx) { diff --git a/src/xrpld/app/tx/detail/NFTokenModify.h b/src/xrpld/app/tx/detail/NFTokenModify.h index 04784381fb..0d1e72ade1 100644 --- a/src/xrpld/app/tx/detail/NFTokenModify.h +++ b/src/xrpld/app/tx/detail/NFTokenModify.h @@ -33,9 +33,6 @@ public: { } - static bool - checkExtraFeatures(PreflightContext const& ctx); - static NotTEC preflight(PreflightContext const& ctx); diff --git a/src/xrpld/app/tx/detail/NFTokenUtils.cpp b/src/xrpld/app/tx/detail/NFTokenUtils.cpp index f246e89e65..9516ba311f 100644 --- a/src/xrpld/app/tx/detail/NFTokenUtils.cpp +++ b/src/xrpld/app/tx/detail/NFTokenUtils.cpp @@ -142,32 +142,26 @@ getPageForToken( // equivalent tokens. This requires special handling. if (splitIter == narr.begin()) { - // Prior to fixNFTokenDirV1 we simply stopped. - if (!view.rules().enabled(fixNFTokenDirV1)) - return nullptr; - else + auto const relation{(id & nft::pageMask) <=> cmp}; + if (relation == 0) { - auto const relation{(id & nft::pageMask) <=> cmp}; - if (relation == 0) - { - // If the passed in id belongs exactly on this (full) page - // this account simply cannot store the NFT. - return nullptr; - } - - if (relation > 0) - { - // We need to leave the entire contents of this page in - // narr so carr stays empty. The new NFT will be - // inserted in carr. This keeps the NFTs that must be - // together all on their own page. - splitIter = narr.end(); - } - - // If neither of those conditions apply then put all of - // narr into carr and produce an empty narr where the new NFT - // will be inserted. Leave the split at narr.begin(). + // If the passed in id belongs exactly on this (full) page + // this account simply cannot store the NFT. + return nullptr; } + + if (relation > 0) + { + // We need to leave the entire contents of this page in + // narr so carr stays empty. The new NFT will be + // inserted in carr. This keeps the NFTs that must be + // together all on their own page. + splitIter = narr.end(); + } + + // If neither of those conditions apply then put all of + // narr into carr and produce an empty narr where the new NFT + // will be inserted. Leave the split at narr.begin(). } // Split narr at splitIter. @@ -178,9 +172,7 @@ getPageForToken( std::swap(carr, newCarr); } - // Determine the ID for the page index. This decision is conditional on - // fixNFTokenDirV1 being enabled. But the condition for the decision - // is not possible unless fixNFTokenDirV1 is enabled. + // Determine the ID for the page index. // // Note that we use uint256::next() because there's a subtlety in the way // NFT pages are structured. The low 96-bits of NFT ID must be strictly @@ -217,13 +209,6 @@ getPageForToken( createCallback(view, owner); - // fixNFTokenDirV1 corrects a bug in the initial implementation that - // would put an NFT in the wrong page. The problem was caused by an - // off-by-one subtlety that the NFT can only be stored in the first page - // with a key that's strictly greater than `first` - if (!view.rules().enabled(fixNFTokenDirV1)) - return (first.key <= np->key()) ? np : cp; - return (first.key < np->key()) ? np : cp; } @@ -844,7 +829,7 @@ tokenOfferCreatePreflight( std::optional const& owner, std::uint32_t txFlags) { - if (amount.negative() && rules.enabled(fixNFTokenNegOffer)) + if (amount.negative()) // An offer for a negative amount makes no sense. return temBAD_AMOUNT; @@ -874,21 +859,10 @@ tokenOfferCreatePreflight( if (owner && owner == acctID) return temMALFORMED; - if (dest) + // The destination can't be the account executing the transaction. + if (dest && dest == acctID) { - // Some folks think it makes sense for a buy offer to specify a - // specific broker using the Destination field. This change doesn't - // deserve it's own amendment, so we're piggy-backing on - // fixNFTokenNegOffer. - // - // Prior to fixNFTokenNegOffer any use of the Destination field on - // a buy offer was malformed. - if (!isSellOffer && !rules.enabled(fixNFTokenNegOffer)) - return temMALFORMED; - - // The destination can't be the account executing the transaction. - if (dest == acctID) - return temMALFORMED; + return temMALFORMED; } return tesSUCCESS; } @@ -946,23 +920,10 @@ tokenOfferCreatePreclaim( // offer may later become unfunded. if ((txFlags & tfSellNFToken) == 0) { - // After this amendment, we allow an IOU issuer to make a buy offer + // We allow an IOU issuer to make a buy offer // using their own currency. - if (view.rules().enabled(fixNonFungibleTokensV1_2)) - { - if (accountFunds( - view, acctID, amount, FreezeHandling::fhZERO_IF_FROZEN, j) - .signum() <= 0) - return tecUNFUNDED_OFFER; - } - else if ( - accountHolds( - view, - acctID, - amount.getCurrency(), - amount.getIssuer(), - FreezeHandling::fhZERO_IF_FROZEN, - j) + if (accountFunds( + view, acctID, amount, FreezeHandling::fhZERO_IF_FROZEN, j) .signum() <= 0) return tecUNFUNDED_OFFER; } diff --git a/src/xrpld/app/tx/detail/SetAccount.cpp b/src/xrpld/app/tx/detail/SetAccount.cpp index 73ddcef974..16bea1e0af 100644 --- a/src/xrpld/app/tx/detail/SetAccount.cpp +++ b/src/xrpld/app/tx/detail/SetAccount.cpp @@ -171,17 +171,14 @@ SetAccount::preflight(PreflightContext const& ctx) return telBAD_DOMAIN; } - if (ctx.rules.enabled(featureNonFungibleTokensV1)) - { - // Configure authorized minting account: - if (uSetFlag == asfAuthorizedNFTokenMinter && - !tx.isFieldPresent(sfNFTokenMinter)) - return temMALFORMED; + // Configure authorized minting account: + if (uSetFlag == asfAuthorizedNFTokenMinter && + !tx.isFieldPresent(sfNFTokenMinter)) + return temMALFORMED; - if (uClearFlag == asfAuthorizedNFTokenMinter && - tx.isFieldPresent(sfNFTokenMinter)) - return temMALFORMED; - } + if (uClearFlag == asfAuthorizedNFTokenMinter && + tx.isFieldPresent(sfNFTokenMinter)) + return temMALFORMED; return tesSUCCESS; } @@ -613,15 +610,12 @@ SetAccount::doApply() } // Configure authorized minting account: - if (ctx_.view().rules().enabled(featureNonFungibleTokensV1)) - { - if (uSetFlag == asfAuthorizedNFTokenMinter) - sle->setAccountID(sfNFTokenMinter, ctx_.tx[sfNFTokenMinter]); + if (uSetFlag == asfAuthorizedNFTokenMinter) + sle->setAccountID(sfNFTokenMinter, ctx_.tx[sfNFTokenMinter]); - if (uClearFlag == asfAuthorizedNFTokenMinter && - sle->isFieldPresent(sfNFTokenMinter)) - sle->makeFieldAbsent(sfNFTokenMinter); - } + if (uClearFlag == asfAuthorizedNFTokenMinter && + sle->isFieldPresent(sfNFTokenMinter)) + sle->makeFieldAbsent(sfNFTokenMinter); // Set or clear flags for disallowing various incoming instruments if (ctx_.view().rules().enabled(featureDisallowIncoming))