From f7a8d2de84c6968c8dad8c842d950dfae7224223 Mon Sep 17 00:00:00 2001 From: ledhed2222 Date: Wed, 18 Jan 2023 16:37:39 -0500 Subject: [PATCH 1/6] Add fixUnburnableNFToken feature (#4391) --- src/ripple/protocol/Feature.h | 3 ++- src/ripple/protocol/impl/Feature.cpp | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 78b5b152c87..0bdfd224dda 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -74,7 +74,7 @@ namespace detail { // Feature.cpp. Because it's only used to reserve storage, and determine how // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than // the actual number of amendments. A LogicError on startup will verify this. -static constexpr std::size_t numFeatures = 56; +static constexpr std::size_t numFeatures = 57; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated @@ -343,6 +343,7 @@ extern uint256 const featureImmediateOfferKilled; extern uint256 const featureDisallowIncoming; extern uint256 const featureXRPFees; extern uint256 const fixUniversalNumber; +extern uint256 const fixUnburnableNFToken; } // namespace ripple diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 2f01c39888a..f021ea4674d 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -453,6 +453,7 @@ REGISTER_FEATURE(ImmediateOfferKilled, Supported::yes, DefaultVote::no) REGISTER_FEATURE(DisallowIncoming, Supported::yes, DefaultVote::no); REGISTER_FEATURE(XRPFees, Supported::yes, DefaultVote::no); REGISTER_FIX (fixUniversalNumber, Supported::yes, DefaultVote::no); +REGISTER_FIX (fixUnburnableNFToken, Supported::yes, DefaultVote::no); // The following amendments have been active for at least two years. Their // pre-amendment code has been removed and the identifiers are deprecated. From a828e24cf0061380524a8c7ef304c52ab3ea100d Mon Sep 17 00:00:00 2001 From: Shawn Xie <35279399+shawnxie999@users.noreply.github.com> Date: Thu, 2 Feb 2023 18:48:03 -0500 Subject: [PATCH 2/6] Allow NFT to be burned when number of offers is greater than 500 (#4346) * Allow offers to be removable * Delete sell offers first Signed-off-by: Shawn Xie --- src/ripple/app/tx/impl/NFTokenBurn.cpp | 46 ++- .../app/tx/impl/details/NFTokenUtils.cpp | 67 ++-- src/ripple/app/tx/impl/details/NFTokenUtils.h | 10 +- src/test/app/NFTokenBurn_test.cpp | 323 ++++++++++++++---- 4 files changed, 347 insertions(+), 99 deletions(-) diff --git a/src/ripple/app/tx/impl/NFTokenBurn.cpp b/src/ripple/app/tx/impl/NFTokenBurn.cpp index da23d78bdbd..e8693c7c6fb 100644 --- a/src/ripple/app/tx/impl/NFTokenBurn.cpp +++ b/src/ripple/app/tx/impl/NFTokenBurn.cpp @@ -77,9 +77,14 @@ NFTokenBurn::preclaim(PreclaimContext const& ctx) } } - // 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]); + if (!ctx.view.rules().enabled(fixUnburnableNFToken)) + { + // 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; } TER @@ -104,9 +109,38 @@ NFTokenBurn::doApply() view().update(issuer); } - // Optimized deletion of all offers. - nft::removeAllTokenOffers(view(), keylet::nft_sells(ctx_.tx[sfNFTokenID])); - nft::removeAllTokenOffers(view(), keylet::nft_buys(ctx_.tx[sfNFTokenID])); + if (ctx_.view().rules().enabled(fixUnburnableNFToken)) + { + // 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 + { + // 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()); + } return tesSUCCESS; } diff --git a/src/ripple/app/tx/impl/details/NFTokenUtils.cpp b/src/ripple/app/tx/impl/details/NFTokenUtils.cpp index d1214a98ee8..db2c3ae62f7 100644 --- a/src/ripple/app/tx/impl/details/NFTokenUtils.cpp +++ b/src/ripple/app/tx/impl/details/NFTokenUtils.cpp @@ -520,34 +520,55 @@ findTokenAndPage( } return std::nullopt; } -void -removeAllTokenOffers(ApplyView& view, Keylet const& directory) -{ - view.dirDelete(directory, [&view](uint256 const& id) { - auto offer = view.peek(Keylet{ltNFTOKEN_OFFER, id}); - if (!offer) - Throw( - "Offer " + to_string(id) + " not found in ledger!"); +std::size_t +removeTokenOffersWithLimit( + ApplyView& view, + Keylet const& directory, + std::size_t maxDeletableOffers) +{ + if (maxDeletableOffers == 0) + return 0; - auto const owner = (*offer)[sfOwner]; + std::optional pageIndex{0}; + std::size_t deletedOffersCount = 0; - if (!view.dirRemove( - keylet::ownerDir(owner), - (*offer)[sfOwnerNode], - offer->key(), - false)) - Throw( - "Offer " + to_string(id) + " not found in owner directory!"); + do + { + auto const page = view.peek(keylet::page(directory, *pageIndex)); + if (!page) + break; + + // We get the index of the next page in case the current + // page is deleted after all of its entries have been removed + pageIndex = (*page)[~sfIndexNext]; + + auto offerIndexes = page->getFieldV256(sfIndexes); + + // We reverse-iterate the offer directory page to delete all entries. + // Deleting an entry in a NFTokenOffer directory page won't cause + // entries from other pages to move to the current, so, it is safe to + // delete entries one by one in the page. It is required to iterate + // backwards to handle iterator invalidation for vector, as we are + // deleting during iteration. + for (int i = offerIndexes.size() - 1; i >= 0; --i) + { + if (auto const offer = view.peek(keylet::nftoffer(offerIndexes[i]))) + { + if (deleteTokenOffer(view, offer)) + ++deletedOffersCount; + else + Throw( + "Offer " + to_string(offerIndexes[i]) + + " cannot be deleted!"); + } - adjustOwnerCount( - view, - view.peek(keylet::account(owner)), - -1, - beast::Journal{beast::Journal::getNullSink()}); + if (maxDeletableOffers == deletedOffersCount) + break; + } + } while (pageIndex.value_or(0) && maxDeletableOffers != deletedOffersCount); - view.erase(offer); - }); + return deletedOffersCount; } TER diff --git a/src/ripple/app/tx/impl/details/NFTokenUtils.h b/src/ripple/app/tx/impl/details/NFTokenUtils.h index fa8c43b5877..db7cf00be10 100644 --- a/src/ripple/app/tx/impl/details/NFTokenUtils.h +++ b/src/ripple/app/tx/impl/details/NFTokenUtils.h @@ -53,9 +53,13 @@ constexpr std::uint16_t const flagOnlyXRP = 0x0002; constexpr std::uint16_t const flagCreateTrustLines = 0x0004; constexpr std::uint16_t const flagTransferable = 0x0008; -/** Deletes all offers from the specified token offer directory. */ -void -removeAllTokenOffers(ApplyView& view, Keylet const& directory); +/** Delete up to a specified number of offers from the specified token offer + * directory. */ +std::size_t +removeTokenOffersWithLimit( + ApplyView& view, + Keylet const& directory, + std::size_t maxDeletableOffers); /** Returns tesSUCCESS if NFToken has few enough offers that it can be burned */ TER diff --git a/src/test/app/NFTokenBurn_test.cpp b/src/test/app/NFTokenBurn_test.cpp index 00124731cb9..4896932acd2 100644 --- a/src/test/app/NFTokenBurn_test.cpp +++ b/src/test/app/NFTokenBurn_test.cpp @@ -49,6 +49,37 @@ class NFTokenBurn_test : public beast::unit_test::suite return nfts[jss::result][jss::account_nfts].size(); }; + // Helper function that returns new nft id for an account and create + // specified number of sell offers + uint256 + createNftAndOffers( + test::jtx::Env& env, + test::jtx::Account const& owner, + std::vector& offerIndexes, + size_t const tokenCancelCount) + { + using namespace test::jtx; + uint256 const nftokenID = + token::getNextID(env, owner, 0, tfTransferable); + env(token::mint(owner, 0), + token::uri(std::string(maxTokenURILength, 'u')), + txflags(tfTransferable)); + env.close(); + + offerIndexes.reserve(tokenCancelCount); + + for (uint32_t i = 0; i < tokenCancelCount; ++i) + { + // Create sell offer + offerIndexes.push_back(keylet::nftoffer(owner, env.seq(owner)).key); + env(token::createOffer(owner, nftokenID, drops(1)), + txflags(tfSellNFToken)); + env.close(); + } + + return nftokenID; + }; + void testBurnRandom(FeatureBitset features) { @@ -492,94 +523,251 @@ class NFTokenBurn_test : public beast::unit_test::suite using namespace test::jtx; - Env env{*this, features}; + // Test what happens if a NFT is unburnable when there are + // more than 500 offers, before fixUnburnableNFToken goes live + if (!features[fixUnburnableNFToken]) + { + Env env{*this, features}; - Account const alice("alice"); - Account const becky("becky"); - env.fund(XRP(1000), alice, becky); - env.close(); + 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 1000 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(); + // 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 (uint32_t i = 0; i < maxTokenOfferCancelCount; ++i) + 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 fixUnburnableNFToken is enabled. This is to test that we + // can successfully remove all offers if the number of offers is less + // than 500. + if (features[fixUnburnableNFToken]) { - Account const acct(std::string("acct") + std::to_string(i)); - env.fund(XRP(1000), acct); + Env env{*this, features}; + + Account const alice("alice"); + Account const becky("becky"); + env.fund(XRP(100000), alice, becky); env.close(); - offerIndexes.push_back(keylet::nftoffer(acct, env.seq(acct)).key); - env(token::createOffer(acct, nftokenID, drops(1)), + // alice creates 498 sell offers and becky creates 1 buy offers. + // When the token is burned, 498 sell offers and 1 buy offer are + // removed. In total, 499 offers are removed + std::vector offerIndexes; + auto const nftokenID = createNftAndOffers( + env, alice, offerIndexes, maxDeletableTokenOfferEntries - 2); + + // Verify all sell offers are present in the ledger. + for (uint256 const& offerIndex : offerIndexes) + { + BEAST_EXPECT(env.le(keylet::nftoffer(offerIndex))); + } + + // Becky creates a buy offer + uint256 const beckyOfferIndex = + keylet::nftoffer(becky, env.seq(becky)).key; + env(token::createOffer(becky, nftokenID, drops(1)), token::owner(alice)); env.close(); + + // Burn the token + env(token::burn(alice, nftokenID)); + env.close(); + + // Burning the token should remove all 498 sell offers + // that alice created + for (uint256 const& offerIndex : offerIndexes) + { + BEAST_EXPECT(!env.le(keylet::nftoffer(offerIndex))); + } + + // Burning the token should also remove the one buy offer + // that becky created + BEAST_EXPECT(!env.le(keylet::nftoffer(beckyOfferIndex))); + + // alice and becky should have ownerCounts of zero + BEAST_EXPECT(ownerCount(env, alice) == 0); + BEAST_EXPECT(ownerCount(env, becky) == 0); } - // Verify all offers are present in the ledger. - for (uint256 const& offerIndex : offerIndexes) + // Test that up to 500 buy offers are removed when NFT is burned + // after fixUnburnableNFToken is enabled + if (features[fixUnburnableNFToken]) { - BEAST_EXPECT(env.le(keylet::nftoffer(offerIndex))); - } + Env env{*this, features}; - // 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)); + Account const alice("alice"); + Account const becky("becky"); + env.fund(XRP(100000), alice, becky); + env.close(); - // Attempt to burn the nft which should fail. - env(token::burn(alice, nftokenID), ter(tefTOO_BIG)); + // alice creates 501 sell offers for the token + // After we burn the token, 500 of the sell offers should be + // removed, and one is left over + std::vector offerIndexes; + auto const nftokenID = createNftAndOffers( + env, alice, offerIndexes, maxDeletableTokenOfferEntries + 1); - // Close enough ledgers that the burn transaction is no longer retried. - for (int i = 0; i < 10; ++i) - env.close(); + // Verify all sell offers are present in the ledger. + for (uint256 const& offerIndex : offerIndexes) + { + BEAST_EXPECT(env.le(keylet::nftoffer(offerIndex))); + } - // Cancel becky's offer, but alice adds a sell offer. The token - // should still not be burnable. - env(token::cancelOffer(becky, {beckyOfferIndex})); - env.close(); + // Burn the token + env(token::burn(alice, nftokenID)); + env.close(); - uint256 const aliceOfferIndex = - keylet::nftoffer(alice, env.seq(alice)).key; - env(token::createOffer(alice, nftokenID, drops(1)), - txflags(tfSellNFToken)); - env.close(); + uint32_t offerDeletedCount = 0; + // Count the number of sell offers that have been deleted + for (uint256 const& offerIndex : offerIndexes) + { + if (!env.le(keylet::nftoffer(offerIndex))) + offerDeletedCount++; + } - env(token::burn(alice, nftokenID), ter(tefTOO_BIG)); - env.close(); + BEAST_EXPECT(offerIndexes.size() == maxTokenOfferCancelCount + 1); - // Cancel alice's sell offer. Now the token should be burnable. - env(token::cancelOffer(alice, {aliceOfferIndex})); - env.close(); + // 500 sell offers should be removed + BEAST_EXPECT(offerDeletedCount == maxTokenOfferCancelCount); - env(token::burn(alice, nftokenID)); - env.close(); + // alice should have ownerCounts of one for the orphaned sell offer + BEAST_EXPECT(ownerCount(env, alice) == 1); + } - // Burning the token should remove all the offers from the ledger. - for (uint256 const& offerIndex : offerIndexes) + // Test that up to 500 buy/sell offers are removed when NFT is burned + // after fixUnburnableNFToken is enabled + if (features[fixUnburnableNFToken]) { - BEAST_EXPECT(!env.le(keylet::nftoffer(offerIndex))); - } + Env env{*this, features}; - // Both alice and becky should have ownerCounts of zero. - BEAST_EXPECT(ownerCount(env, alice) == 0); - BEAST_EXPECT(ownerCount(env, becky) == 0); + Account const alice("alice"); + Account const becky("becky"); + env.fund(XRP(100000), alice, becky); + env.close(); + + // alice creates 499 sell offers and becky creates 2 buy offers. + // When the token is burned, 499 sell offers and 1 buy offer + // are removed. + // In total, 500 offers are removed + std::vector offerIndexes; + auto const nftokenID = createNftAndOffers( + env, alice, offerIndexes, maxDeletableTokenOfferEntries - 1); + + // Verify all sell offers are present in the ledger. + for (uint256 const& offerIndex : offerIndexes) + { + BEAST_EXPECT(env.le(keylet::nftoffer(offerIndex))); + } + + // becky creates 2 buy offers + env(token::createOffer(becky, nftokenID, drops(1)), + token::owner(alice)); + env.close(); + env(token::createOffer(becky, nftokenID, drops(1)), + token::owner(alice)); + env.close(); + + // Burn the token + env(token::burn(alice, nftokenID)); + env.close(); + + // Burning the token should remove all 499 sell offers from the + // ledger. + for (uint256 const& offerIndex : offerIndexes) + { + BEAST_EXPECT(!env.le(keylet::nftoffer(offerIndex))); + } + + // alice should have ownerCount of zero because all her + // sell offers have been deleted + BEAST_EXPECT(ownerCount(env, alice) == 0); + + // becky has ownerCount of one due to an orphaned buy offer + BEAST_EXPECT(ownerCount(env, becky) == 1); + } } void @@ -598,7 +786,8 @@ class NFTokenBurn_test : public beast::unit_test::suite FeatureBitset const all{supported_amendments()}; FeatureBitset const fixNFTDir{fixNFTokenDirV1}; - testWithFeats(all - fixNFTDir); + testWithFeats(all - fixUnburnableNFToken - fixNFTDir); + testWithFeats(all - fixUnburnableNFToken); testWithFeats(all); } }; From 89aa8b21ec14233399513b60c88abd2969f2cba7 Mon Sep 17 00:00:00 2001 From: ledhed2222 Date: Thu, 9 Feb 2023 16:57:51 -0500 Subject: [PATCH 3/6] Fix 3 issues around NFToken offer acceptance (#4380) Fixes 3 issues: In the following scenario, an account cannot perform NFTokenAcceptOffer even though it should be allowed to: - BROKER has < S - ALICE offers to sell token for S - BOB offers to buy token for > S - BROKER tries to bridge the two offers This currently results in `tecINSUFFICIENT_FUNDS`, but should not because BROKER is not spending any funds in this transaction, beyond the transaction fee. When trading an NFT using IOUs, and when the issuer of the IOU has any non-zero value set for TransferFee on their account via AccountSet (not a TransferFee on the NFT), and when the sale amount is equal to the total balance of that IOU that the buyer has, the resulting balance for the issuer of the IOU will become positive. This means that the buyer of the NFT was supposed to have caused a certain amount of IOU to be burned. That amount was unable to be burned because the buyer couldn't cover it. This results in the buyer owing this amount back to the issuer. In a real world scenario, this is appropriate and can be settled off-chain. Currency issuers could not make offers for NFTs using their own currency, receiving `tecINSUFFICIENT_FUNDS` if they tried to do so. With this fix, they are now able to buy/sell NFTs using their own currency. --- src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp | 74 +- src/ripple/app/tx/impl/NFTokenCreateOffer.cpp | 31 +- src/ripple/ledger/View.h | 5 + src/test/app/NFToken_test.cpp | 1650 +++++++++++++---- 4 files changed, 1339 insertions(+), 421 deletions(-) diff --git a/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp b/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp index 07fe9957a76..c335f8d28fd 100644 --- a/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp +++ b/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp @@ -168,10 +168,21 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx) dest.has_value() && *dest != ctx.tx[sfAccount]) return tecNO_PERMISSION; } + // The account offering to buy must have funds: + // + // After this amendment, we allow an IOU issuer to buy an NFT with their + // own currency auto const needed = bo->at(sfAmount); - - if (accountHolds( + if (ctx.view.rules().enabled(fixUnburnableNFToken)) + { + 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(), @@ -206,15 +217,39 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx) // The account offering to buy must have funds: auto const needed = so->at(sfAmount); - - if (accountHolds( - ctx.view, - ctx.tx[sfAccount], - needed.getCurrency(), - needed.getIssuer(), - fhZERO_IF_FROZEN, - ctx.j) < needed) - return tecINSUFFICIENT_FUNDS; + if (!ctx.view.rules().enabled(fixUnburnableNFToken)) + { + if (accountHolds( + ctx.view, + ctx.tx[sfAccount], + needed.getCurrency(), + needed.getIssuer(), + fhZERO_IF_FROZEN, + ctx.j) < needed) + return tecINSUFFICIENT_FUNDS; + } + else if (!bo) + { + // After this amendment, we allow buyers to buy with their own + // issued currency. + // + // In the case of brokered mode, this check is essentially + // redundant, since we have already confirmed that buy offer is > + // than the sell offer, and that the buyer can cover the buy + // offer. + // + // We also _must not_ check the tx submitter in brokered + // mode, because then we are confirming that the broker can + // cover what the buyer will pay, which doesn't make sense, causes + // an unncessary tec, and is also resolved with this amendment. + if (accountFunds( + ctx.view, + ctx.tx[sfAccount], + needed, + fhZERO_IF_FROZEN, + ctx.j) < needed) + return tecINSUFFICIENT_FUNDS; + } } return tesSUCCESS; @@ -230,7 +265,22 @@ NFTokenAcceptOffer::pay( if (amount < beast::zero) return tecINTERNAL; - return accountSend(view(), from, to, amount, j_); + 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(fixUnburnableNFToken)) + return result; + if (result != tesSUCCESS) + return result; + if (accountFunds(view(), from, amount, fhZERO_IF_FROZEN, j_).signum() < 0) + return tecINSUFFICIENT_FUNDS; + if (accountFunds(view(), to, amount, fhZERO_IF_FROZEN, j_).signum() < 0) + return tecINSUFFICIENT_FUNDS; + return tesSUCCESS; } TER diff --git a/src/ripple/app/tx/impl/NFTokenCreateOffer.cpp b/src/ripple/app/tx/impl/NFTokenCreateOffer.cpp index 695efdd0aa4..ff8668e4488 100644 --- a/src/ripple/app/tx/impl/NFTokenCreateOffer.cpp +++ b/src/ripple/app/tx/impl/NFTokenCreateOffer.cpp @@ -153,15 +153,28 @@ NFTokenCreateOffer::preclaim(PreclaimContext const& ctx) // offer may later become unfunded. if (!isSellOffer) { - auto const funds = accountHolds( - ctx.view, - ctx.tx[sfAccount], - amount.getCurrency(), - amount.getIssuer(), - FreezeHandling::fhZERO_IF_FROZEN, - ctx.j); - - if (funds.signum() <= 0) + // After this amendment, we allow an IOU issuer to make a buy offer + // using their own currency. + if (ctx.view.rules().enabled(fixUnburnableNFToken)) + { + if (accountFunds( + ctx.view, + ctx.tx[sfAccount], + amount, + FreezeHandling::fhZERO_IF_FROZEN, + ctx.j) + .signum() <= 0) + return tecUNFUNDED_OFFER; + } + else if ( + accountHolds( + ctx.view, + ctx.tx[sfAccount], + amount.getCurrency(), + amount.getIssuer(), + FreezeHandling::fhZERO_IF_FROZEN, + ctx.j) + .signum() <= 0) return tecUNFUNDED_OFFER; } diff --git a/src/ripple/ledger/View.h b/src/ripple/ledger/View.h index ee917115515..24a647c768d 100644 --- a/src/ripple/ledger/View.h +++ b/src/ripple/ledger/View.h @@ -97,6 +97,11 @@ accountHolds( FreezeHandling zeroIfFrozen, beast::Journal j); +// Returns the amount an account can spend of the currency type saDefault, or +// returns saDefault if this account is the issuer of the the currency in +// question. Should be used in favor of accountHolds when questioning how much +// an account can spend while also allowing currency issuers to spend +// unlimited amounts of their own currency (since they can always issue more). [[nodiscard]] STAmount accountFunds( ReadView const& view, diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 842f3f76cc8..33d725e5a17 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -3827,497 +3827,586 @@ class NFToken_test : public beast::unit_test::suite using namespace test::jtx; - Env env{*this, features}; - - // The most important thing to explore here is the way funds are - // assigned from the buyer to... - // o the Seller, - // o the Broker, and - // o the Issuer (in the case of a transfer fee). - - Account const issuer{"issuer"}; - Account const minter{"minter"}; - Account const buyer{"buyer"}; - Account const broker{"broker"}; - Account const gw{"gw"}; - IOU const gwXAU(gw["XAU"]); - - env.fund(XRP(1000), issuer, minter, buyer, broker, gw); - env.close(); - - env(trust(issuer, gwXAU(2000))); - env(trust(minter, gwXAU(2000))); - env(trust(buyer, gwXAU(2000))); - env(trust(broker, gwXAU(2000))); - env.close(); - - env(token::setMinter(issuer, minter)); - env.close(); - - // Lambda to check owner count of all accounts is one. - auto checkOwnerCountIsOne = - [this, &env]( - std::initializer_list> - accounts, - int line) { - for (Account const& acct : accounts) - { - if (std::uint32_t ownerCount = this->ownerCount(env, acct); - ownerCount != 1) - { - std::stringstream ss; - ss << "Account " << acct.human() - << " expected ownerCount == 1. Got " << ownerCount; - fail(ss.str(), __FILE__, line); - } - } - }; - - // Lambda that mints an NFT and returns the nftID. - auto mintNFT = [&env, &issuer, &minter](std::uint16_t xferFee = 0) { - uint256 const nftID = - token::getNextID(env, issuer, 0, tfTransferable, xferFee); - env(token::mint(minter, 0), - token::issuer(issuer), - token::xferFee(xferFee), - txflags(tfTransferable)); - env.close(); - return nftID; - }; - - // o Seller is selling for zero XRP. - // o Broker charges no fee. - // o No transfer fee. - // - // Since minter is selling for zero the currency must be XRP. + for (auto const& tweakedFeatures : + {features - fixUnburnableNFToken, features | fixUnburnableNFToken}) { - checkOwnerCountIsOne({issuer, minter, buyer, broker}, __LINE__); + Env env{*this, tweakedFeatures}; - uint256 const nftID = mintNFT(); + // The most important thing to explore here is the way funds are + // assigned from the buyer to... + // o the Seller, + // o the Broker, and + // o the Issuer (in the case of a transfer fee). - // minter creates their offer. - uint256 const minterOfferIndex = - keylet::nftoffer(minter, env.seq(minter)).key; - env(token::createOffer(minter, nftID, XRP(0)), - txflags(tfSellNFToken)); - env.close(); + Account const issuer{"issuer"}; + Account const minter{"minter"}; + Account const buyer{"buyer"}; + Account const broker{"broker"}; + Account const gw{"gw"}; + IOU const gwXAU(gw["XAU"]); - // buyer creates their offer. Note: a buy offer can never - // offer zero. - uint256 const buyOfferIndex = - keylet::nftoffer(buyer, env.seq(buyer)).key; - env(token::createOffer(buyer, nftID, XRP(1)), token::owner(minter)); + env.fund(XRP(1000), issuer, minter, buyer, broker, gw); env.close(); - auto const minterBalance = env.balance(minter); - auto const buyerBalance = env.balance(buyer); - auto const brokerBalance = env.balance(broker); - auto const issuerBalance = env.balance(issuer); - - // Broker charges no brokerFee. - env(token::brokerOffers(broker, buyOfferIndex, minterOfferIndex)); + env(trust(issuer, gwXAU(2000))); + env(trust(minter, gwXAU(2000))); + env(trust(buyer, gwXAU(2000))); + env(trust(broker, gwXAU(2000))); env.close(); - // Note that minter's XRP balance goes up even though they - // requested XRP(0). - BEAST_EXPECT(env.balance(minter) == minterBalance + XRP(1)); - BEAST_EXPECT(env.balance(buyer) == buyerBalance - XRP(1)); - BEAST_EXPECT(env.balance(broker) == brokerBalance - drops(10)); - BEAST_EXPECT(env.balance(issuer) == issuerBalance); - - // Burn the NFT so the next test starts with a clean state. - env(token::burn(buyer, nftID)); + env(token::setMinter(issuer, minter)); env.close(); - } - // o Seller is selling for zero XRP. - // o Broker charges a fee. - // o No transfer fee. - // - // Since minter is selling for zero the currency must be XRP. - { - checkOwnerCountIsOne({issuer, minter, buyer, broker}, __LINE__); + // Lambda to check owner count of all accounts is one. + auto checkOwnerCountIsOne = + [this, &env]( + std::initializer_list> + accounts, + int line) { + for (Account const& acct : accounts) + { + if (std::uint32_t ownerCount = + this->ownerCount(env, acct); + ownerCount != 1) + { + std::stringstream ss; + ss << "Account " << acct.human() + << " expected ownerCount == 1. Got " + << ownerCount; + fail(ss.str(), __FILE__, line); + } + } + }; - uint256 const nftID = mintNFT(); + // Lambda that mints an NFT and returns the nftID. + auto mintNFT = [&env, &issuer, &minter](std::uint16_t xferFee = 0) { + uint256 const nftID = + token::getNextID(env, issuer, 0, tfTransferable, xferFee); + env(token::mint(minter, 0), + token::issuer(issuer), + token::xferFee(xferFee), + txflags(tfTransferable)); + env.close(); + return nftID; + }; - // minter creates their offer. - uint256 const minterOfferIndex = - keylet::nftoffer(minter, env.seq(minter)).key; - env(token::createOffer(minter, nftID, XRP(0)), - txflags(tfSellNFToken)); - env.close(); + // o Seller is selling for zero XRP. + // o Broker charges no fee. + // o No transfer fee. + // + // Since minter is selling for zero the currency must be XRP. + { + checkOwnerCountIsOne({issuer, minter, buyer, broker}, __LINE__); - // buyer creates their offer. Note: a buy offer can never - // offer zero. - uint256 const buyOfferIndex = - keylet::nftoffer(buyer, env.seq(buyer)).key; - env(token::createOffer(buyer, nftID, XRP(1)), token::owner(minter)); - env.close(); + uint256 const nftID = mintNFT(); - // Broker attempts to charge a 1.1 XRP brokerFee and fails. - env(token::brokerOffers(broker, buyOfferIndex, minterOfferIndex), - token::brokerFee(XRP(1.1)), - ter(tecINSUFFICIENT_PAYMENT)); - env.close(); + // minter creates their offer. + uint256 const minterOfferIndex = + keylet::nftoffer(minter, env.seq(minter)).key; + env(token::createOffer(minter, nftID, XRP(0)), + txflags(tfSellNFToken)); + env.close(); - auto const minterBalance = env.balance(minter); - auto const buyerBalance = env.balance(buyer); - auto const brokerBalance = env.balance(broker); - auto const issuerBalance = env.balance(issuer); + // buyer creates their offer. Note: a buy offer can never + // offer zero. + uint256 const buyOfferIndex = + keylet::nftoffer(buyer, env.seq(buyer)).key; + env(token::createOffer(buyer, nftID, XRP(1)), + token::owner(minter)); + env.close(); - // Broker charges a 0.5 XRP brokerFee. - env(token::brokerOffers(broker, buyOfferIndex, minterOfferIndex), - token::brokerFee(XRP(0.5))); - env.close(); + auto const minterBalance = env.balance(minter); + auto const buyerBalance = env.balance(buyer); + auto const brokerBalance = env.balance(broker); + auto const issuerBalance = env.balance(issuer); - // Note that minter's XRP balance goes up even though they - // requested XRP(0). - BEAST_EXPECT(env.balance(minter) == minterBalance + XRP(0.5)); - BEAST_EXPECT(env.balance(buyer) == buyerBalance - XRP(1)); - BEAST_EXPECT( - env.balance(broker) == brokerBalance + XRP(0.5) - drops(10)); - BEAST_EXPECT(env.balance(issuer) == issuerBalance); + // Broker charges no brokerFee. + env(token::brokerOffers( + broker, buyOfferIndex, minterOfferIndex)); + env.close(); - // Burn the NFT so the next test starts with a clean state. - env(token::burn(buyer, nftID)); - env.close(); - } + // Note that minter's XRP balance goes up even though they + // requested XRP(0). + BEAST_EXPECT(env.balance(minter) == minterBalance + XRP(1)); + BEAST_EXPECT(env.balance(buyer) == buyerBalance - XRP(1)); + BEAST_EXPECT(env.balance(broker) == brokerBalance - drops(10)); + BEAST_EXPECT(env.balance(issuer) == issuerBalance); - // o Seller is selling for zero XRP. - // o Broker charges no fee. - // o 50% transfer fee. - // - // Since minter is selling for zero the currency must be XRP. - { - checkOwnerCountIsOne({issuer, minter, buyer, broker}, __LINE__); + // Burn the NFT so the next test starts with a clean state. + env(token::burn(buyer, nftID)); + env.close(); + } - uint256 const nftID = mintNFT(maxTransferFee); + // o Seller is selling for zero XRP. + // o Broker charges a fee. + // o No transfer fee. + // + // Since minter is selling for zero the currency must be XRP. + { + checkOwnerCountIsOne({issuer, minter, buyer, broker}, __LINE__); - // minter creates their offer. - uint256 const minterOfferIndex = - keylet::nftoffer(minter, env.seq(minter)).key; - env(token::createOffer(minter, nftID, XRP(0)), - txflags(tfSellNFToken)); - env.close(); + uint256 const nftID = mintNFT(); - // buyer creates their offer. Note: a buy offer can never - // offer zero. - uint256 const buyOfferIndex = - keylet::nftoffer(buyer, env.seq(buyer)).key; - env(token::createOffer(buyer, nftID, XRP(1)), token::owner(minter)); - env.close(); + // minter creates their offer. + uint256 const minterOfferIndex = + keylet::nftoffer(minter, env.seq(minter)).key; + env(token::createOffer(minter, nftID, XRP(0)), + txflags(tfSellNFToken)); + env.close(); - auto const minterBalance = env.balance(minter); - auto const buyerBalance = env.balance(buyer); - auto const brokerBalance = env.balance(broker); - auto const issuerBalance = env.balance(issuer); + // buyer creates their offer. Note: a buy offer can never + // offer zero. + uint256 const buyOfferIndex = + keylet::nftoffer(buyer, env.seq(buyer)).key; + env(token::createOffer(buyer, nftID, XRP(1)), + token::owner(minter)); + env.close(); - // Broker charges no brokerFee. - env(token::brokerOffers(broker, buyOfferIndex, minterOfferIndex)); - env.close(); + // Broker attempts to charge a 1.1 XRP brokerFee and fails. + env(token::brokerOffers( + broker, buyOfferIndex, minterOfferIndex), + token::brokerFee(XRP(1.1)), + ter(tecINSUFFICIENT_PAYMENT)); + env.close(); - // Note that minter's XRP balance goes up even though they - // requested XRP(0). - BEAST_EXPECT(env.balance(minter) == minterBalance + XRP(0.5)); - BEAST_EXPECT(env.balance(buyer) == buyerBalance - XRP(1)); - BEAST_EXPECT(env.balance(broker) == brokerBalance - drops(10)); - BEAST_EXPECT(env.balance(issuer) == issuerBalance + XRP(0.5)); + auto const minterBalance = env.balance(minter); + auto const buyerBalance = env.balance(buyer); + auto const brokerBalance = env.balance(broker); + auto const issuerBalance = env.balance(issuer); - // Burn the NFT so the next test starts with a clean state. - env(token::burn(buyer, nftID)); - env.close(); - } + // Broker charges a 0.5 XRP brokerFee. + env(token::brokerOffers( + broker, buyOfferIndex, minterOfferIndex), + token::brokerFee(XRP(0.5))); + env.close(); - // o Seller is selling for zero XRP. - // o Broker charges 0.5 XRP. - // o 50% transfer fee. - // - // Since minter is selling for zero the currency must be XRP. - { - checkOwnerCountIsOne({issuer, minter, buyer, broker}, __LINE__); + // Note that minter's XRP balance goes up even though they + // requested XRP(0). + BEAST_EXPECT(env.balance(minter) == minterBalance + XRP(0.5)); + BEAST_EXPECT(env.balance(buyer) == buyerBalance - XRP(1)); + BEAST_EXPECT( + env.balance(broker) == + brokerBalance + XRP(0.5) - drops(10)); + BEAST_EXPECT(env.balance(issuer) == issuerBalance); + + // Burn the NFT so the next test starts with a clean state. + env(token::burn(buyer, nftID)); + env.close(); + } - uint256 const nftID = mintNFT(maxTransferFee); + // o Seller is selling for zero XRP. + // o Broker charges no fee. + // o 50% transfer fee. + // + // Since minter is selling for zero the currency must be XRP. + { + checkOwnerCountIsOne({issuer, minter, buyer, broker}, __LINE__); - // minter creates their offer. - uint256 const minterOfferIndex = - keylet::nftoffer(minter, env.seq(minter)).key; - env(token::createOffer(minter, nftID, XRP(0)), - txflags(tfSellNFToken)); - env.close(); + uint256 const nftID = mintNFT(maxTransferFee); - // buyer creates their offer. Note: a buy offer can never - // offer zero. - uint256 const buyOfferIndex = - keylet::nftoffer(buyer, env.seq(buyer)).key; - env(token::createOffer(buyer, nftID, XRP(1)), token::owner(minter)); - env.close(); + // minter creates their offer. + uint256 const minterOfferIndex = + keylet::nftoffer(minter, env.seq(minter)).key; + env(token::createOffer(minter, nftID, XRP(0)), + txflags(tfSellNFToken)); + env.close(); - auto const minterBalance = env.balance(minter); - auto const buyerBalance = env.balance(buyer); - auto const brokerBalance = env.balance(broker); - auto const issuerBalance = env.balance(issuer); + // buyer creates their offer. Note: a buy offer can never + // offer zero. + uint256 const buyOfferIndex = + keylet::nftoffer(buyer, env.seq(buyer)).key; + env(token::createOffer(buyer, nftID, XRP(1)), + token::owner(minter)); + env.close(); - // Broker charges a 0.75 XRP brokerFee. - env(token::brokerOffers(broker, buyOfferIndex, minterOfferIndex), - token::brokerFee(XRP(0.75))); - env.close(); + auto const minterBalance = env.balance(minter); + auto const buyerBalance = env.balance(buyer); + auto const brokerBalance = env.balance(broker); + auto const issuerBalance = env.balance(issuer); - // Note that, with a 50% transfer fee, issuer gets 1/2 of what's - // left _after_ broker takes their fee. minter gets the remainder - // after both broker and minter take their cuts - BEAST_EXPECT(env.balance(minter) == minterBalance + XRP(0.125)); - BEAST_EXPECT(env.balance(buyer) == buyerBalance - XRP(1)); - BEAST_EXPECT( - env.balance(broker) == brokerBalance + XRP(0.75) - drops(10)); - BEAST_EXPECT(env.balance(issuer) == issuerBalance + XRP(0.125)); + // Broker charges no brokerFee. + env(token::brokerOffers( + broker, buyOfferIndex, minterOfferIndex)); + env.close(); - // Burn the NFT so the next test starts with a clean state. - env(token::burn(buyer, nftID)); - env.close(); - } + // Note that minter's XRP balance goes up even though they + // requested XRP(0). + BEAST_EXPECT(env.balance(minter) == minterBalance + XRP(0.5)); + BEAST_EXPECT(env.balance(buyer) == buyerBalance - XRP(1)); + BEAST_EXPECT(env.balance(broker) == brokerBalance - drops(10)); + BEAST_EXPECT(env.balance(issuer) == issuerBalance + XRP(0.5)); - // Lambda to set the balance of all passed in accounts to gwXAU(1000). - auto setXAUBalance_1000 = - [this, &gw, &gwXAU, &env]( - std::initializer_list> - accounts, - int line) { - for (Account const& acct : accounts) - { - static const auto xau1000 = gwXAU(1000); - auto const balance = env.balance(acct, gwXAU); - if (balance < xau1000) - { - env(pay(gw, acct, xau1000 - balance)); - env.close(); - } - else if (balance > xau1000) - { - env(pay(acct, gw, balance - xau1000)); - env.close(); - } - if (env.balance(acct, gwXAU) != xau1000) - { - std::stringstream ss; - ss << "Unable to set " << acct.human() - << " account balance to gwXAU(1000)"; - this->fail(ss.str(), __FILE__, line); - } - } - }; + // Burn the NFT so the next test starts with a clean state. + env(token::burn(buyer, nftID)); + env.close(); + } - // The buyer and seller have identical amounts and there is no - // transfer fee. - { - checkOwnerCountIsOne({issuer, minter, buyer, broker}, __LINE__); - setXAUBalance_1000({issuer, minter, buyer, broker}, __LINE__); + // o Seller is selling for zero XRP. + // o Broker charges 0.5 XRP. + // o 50% transfer fee. + // + // Since minter is selling for zero the currency must be XRP. + { + checkOwnerCountIsOne({issuer, minter, buyer, broker}, __LINE__); - uint256 const nftID = mintNFT(); + uint256 const nftID = mintNFT(maxTransferFee); - // minter creates their offer. - uint256 const minterOfferIndex = - keylet::nftoffer(minter, env.seq(minter)).key; - env(token::createOffer(minter, nftID, gwXAU(1000)), - txflags(tfSellNFToken)); - env.close(); + // minter creates their offer. + uint256 const minterOfferIndex = + keylet::nftoffer(minter, env.seq(minter)).key; + env(token::createOffer(minter, nftID, XRP(0)), + txflags(tfSellNFToken)); + env.close(); - { - // buyer creates an offer for more XAU than they currently own. + // buyer creates their offer. Note: a buy offer can never + // offer zero. uint256 const buyOfferIndex = keylet::nftoffer(buyer, env.seq(buyer)).key; - env(token::createOffer(buyer, nftID, gwXAU(1001)), + env(token::createOffer(buyer, nftID, XRP(1)), token::owner(minter)); env.close(); - // broker attempts to broker the offers but cannot. + auto const minterBalance = env.balance(minter); + auto const buyerBalance = env.balance(buyer); + auto const brokerBalance = env.balance(broker); + auto const issuerBalance = env.balance(issuer); + + // Broker charges a 0.75 XRP brokerFee. env(token::brokerOffers( broker, buyOfferIndex, minterOfferIndex), - ter(tecINSUFFICIENT_FUNDS)); + token::brokerFee(XRP(0.75))); env.close(); - // Cancel buyer's bad offer so the next test starts in a - // clean state. - env(token::cancelOffer(buyer, {buyOfferIndex})); + // Note that, with a 50% transfer fee, issuer gets 1/2 of what's + // left _after_ broker takes their fee. minter gets the + // remainder after both broker and minter take their cuts + BEAST_EXPECT(env.balance(minter) == minterBalance + XRP(0.125)); + BEAST_EXPECT(env.balance(buyer) == buyerBalance - XRP(1)); + BEAST_EXPECT( + env.balance(broker) == + brokerBalance + XRP(0.75) - drops(10)); + BEAST_EXPECT(env.balance(issuer) == issuerBalance + XRP(0.125)); + + // Burn the NFT so the next test starts with a clean state. + env(token::burn(buyer, nftID)); env.close(); } + + // Lambda to set the balance of all passed in accounts to + // gwXAU(amount). + auto setXAUBalance = + [this, &gw, &gwXAU, &env]( + std::initializer_list> + accounts, + int amount, + int line) { + for (Account const& acct : accounts) + { + auto const xauAmt = gwXAU(amount); + auto const balance = env.balance(acct, gwXAU); + if (balance < xauAmt) + { + env(pay(gw, acct, xauAmt - balance)); + env.close(); + } + else if (balance > xauAmt) + { + env(pay(acct, gw, balance - xauAmt)); + env.close(); + } + if (env.balance(acct, gwXAU) != xauAmt) + { + std::stringstream ss; + ss << "Unable to set " << acct.human() + << " account balance to gwXAU(" << amount << ")"; + this->fail(ss.str(), __FILE__, line); + } + } + }; + + // The buyer and seller have identical amounts and there is no + // transfer fee. { - // buyer creates an offer for less that what minter is asking. + checkOwnerCountIsOne({issuer, minter, buyer, broker}, __LINE__); + setXAUBalance({issuer, minter, buyer, broker}, 1000, __LINE__); + + uint256 const nftID = mintNFT(); + + // minter creates their offer. + uint256 const minterOfferIndex = + keylet::nftoffer(minter, env.seq(minter)).key; + env(token::createOffer(minter, nftID, gwXAU(1000)), + txflags(tfSellNFToken)); + env.close(); + + { + // buyer creates an offer for more XAU than they currently + // own. + uint256 const buyOfferIndex = + keylet::nftoffer(buyer, env.seq(buyer)).key; + env(token::createOffer(buyer, nftID, gwXAU(1001)), + token::owner(minter)); + env.close(); + + // broker attempts to broker the offers but cannot. + env(token::brokerOffers( + broker, buyOfferIndex, minterOfferIndex), + ter(tecINSUFFICIENT_FUNDS)); + env.close(); + + // Cancel buyer's bad offer so the next test starts in a + // clean state. + env(token::cancelOffer(buyer, {buyOfferIndex})); + env.close(); + } + { + // buyer creates an offer for less that what minter is + // asking. + uint256 const buyOfferIndex = + keylet::nftoffer(buyer, env.seq(buyer)).key; + env(token::createOffer(buyer, nftID, gwXAU(999)), + token::owner(minter)); + env.close(); + + // broker attempts to broker the offers but cannot. + env(token::brokerOffers( + broker, buyOfferIndex, minterOfferIndex), + ter(tecINSUFFICIENT_PAYMENT)); + env.close(); + + // Cancel buyer's bad offer so the next test starts in a + // clean state. + env(token::cancelOffer(buyer, {buyOfferIndex})); + env.close(); + } + + // buyer creates a large enough offer. uint256 const buyOfferIndex = keylet::nftoffer(buyer, env.seq(buyer)).key; - env(token::createOffer(buyer, nftID, gwXAU(999)), + env(token::createOffer(buyer, nftID, gwXAU(1000)), token::owner(minter)); env.close(); - // broker attempts to broker the offers but cannot. + // Broker attempts to charge a brokerFee but cannot. env(token::brokerOffers( broker, buyOfferIndex, minterOfferIndex), + token::brokerFee(gwXAU(0.1)), ter(tecINSUFFICIENT_PAYMENT)); env.close(); - // Cancel buyer's bad offer so the next test starts in a - // clean state. - env(token::cancelOffer(buyer, {buyOfferIndex})); + // broker charges no brokerFee and succeeds. + env(token::brokerOffers( + broker, buyOfferIndex, minterOfferIndex)); + 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(1000)); + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(2000)); + BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(0)); + BEAST_EXPECT(env.balance(broker, gwXAU) == gwXAU(1000)); + + // Burn the NFT so the next test starts with a clean state. + env(token::burn(buyer, nftID)); env.close(); } - // buyer creates a large enough offer. - uint256 const buyOfferIndex = - keylet::nftoffer(buyer, env.seq(buyer)).key; - env(token::createOffer(buyer, nftID, gwXAU(1000)), - token::owner(minter)); - env.close(); + // seller offers more than buyer is asking. + // There are both transfer and broker fees. + { + checkOwnerCountIsOne({issuer, minter, buyer, broker}, __LINE__); + setXAUBalance({issuer, minter, buyer, broker}, 1000, __LINE__); - // Broker attempts to charge a brokerFee but cannot. - env(token::brokerOffers(broker, buyOfferIndex, minterOfferIndex), - token::brokerFee(gwXAU(0.1)), - ter(tecINSUFFICIENT_PAYMENT)); - env.close(); + uint256 const nftID = mintNFT(maxTransferFee); - // broker charges no brokerFee and succeeds. - env(token::brokerOffers(broker, buyOfferIndex, minterOfferIndex)); - 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(1000)); - BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(2000)); - BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(0)); - BEAST_EXPECT(env.balance(broker, gwXAU) == gwXAU(1000)); + // minter creates their offer. + uint256 const minterOfferIndex = + keylet::nftoffer(minter, env.seq(minter)).key; + env(token::createOffer(minter, nftID, gwXAU(900)), + txflags(tfSellNFToken)); + env.close(); + { + // buyer creates an offer for more XAU than they currently + // own. + uint256 const buyOfferIndex = + keylet::nftoffer(buyer, env.seq(buyer)).key; + env(token::createOffer(buyer, nftID, gwXAU(1001)), + token::owner(minter)); + env.close(); - // Burn the NFT so the next test starts with a clean state. - env(token::burn(buyer, nftID)); - env.close(); - } + // broker attempts to broker the offers but cannot. + env(token::brokerOffers( + broker, buyOfferIndex, minterOfferIndex), + ter(tecINSUFFICIENT_FUNDS)); + env.close(); - // seller offers more than buyer is asking. - // There are both transfer and broker fees. - { - checkOwnerCountIsOne({issuer, minter, buyer, broker}, __LINE__); - setXAUBalance_1000({issuer, minter, buyer, broker}, __LINE__); + // Cancel buyer's bad offer so the next test starts in a + // clean state. + env(token::cancelOffer(buyer, {buyOfferIndex})); + env.close(); + } + { + // buyer creates an offer for less that what minter is + // asking. + uint256 const buyOfferIndex = + keylet::nftoffer(buyer, env.seq(buyer)).key; + env(token::createOffer(buyer, nftID, gwXAU(899)), + token::owner(minter)); + env.close(); - uint256 const nftID = mintNFT(maxTransferFee); + // broker attempts to broker the offers but cannot. + env(token::brokerOffers( + broker, buyOfferIndex, minterOfferIndex), + ter(tecINSUFFICIENT_PAYMENT)); + env.close(); - // minter creates their offer. - uint256 const minterOfferIndex = - keylet::nftoffer(minter, env.seq(minter)).key; - env(token::createOffer(minter, nftID, gwXAU(900)), - txflags(tfSellNFToken)); - env.close(); - { - // buyer creates an offer for more XAU than they currently own. + // Cancel buyer's bad offer so the next test starts in a + // clean state. + env(token::cancelOffer(buyer, {buyOfferIndex})); + env.close(); + } + // buyer creates a large enough offer. uint256 const buyOfferIndex = keylet::nftoffer(buyer, env.seq(buyer)).key; - env(token::createOffer(buyer, nftID, gwXAU(1001)), + env(token::createOffer(buyer, nftID, gwXAU(1000)), token::owner(minter)); env.close(); - // broker attempts to broker the offers but cannot. + // Broker attempts to charge a brokerFee larger than the + // difference between the two offers but cannot. + env(token::brokerOffers( + broker, buyOfferIndex, minterOfferIndex), + token::brokerFee(gwXAU(101)), + ter(tecINSUFFICIENT_PAYMENT)); + env.close(); + + // broker charges the full difference between the two offers and + // succeeds. env(token::brokerOffers( broker, buyOfferIndex, minterOfferIndex), - ter(tecINSUFFICIENT_FUNDS)); + token::brokerFee(gwXAU(100))); env.close(); - // Cancel buyer's bad offer so the next test starts in a - // clean state. - env(token::cancelOffer(buyer, {buyOfferIndex})); + 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(1450)); + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(1450)); + BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(0)); + BEAST_EXPECT(env.balance(broker, gwXAU) == gwXAU(1100)); + + // Burn the NFT so the next test starts with a clean state. + env(token::burn(buyer, nftID)); env.close(); } + // seller offers more than buyer is asking. + // There are both transfer and broker fees, but broker takes less + // than the maximum. { - // buyer creates an offer for less that what minter is asking. + checkOwnerCountIsOne({issuer, minter, buyer, broker}, __LINE__); + setXAUBalance({issuer, minter, buyer, broker}, 1000, __LINE__); + + uint256 const nftID = mintNFT(maxTransferFee / 2); // 25% + + // minter creates their offer. + uint256 const minterOfferIndex = + keylet::nftoffer(minter, env.seq(minter)).key; + env(token::createOffer(minter, nftID, gwXAU(900)), + txflags(tfSellNFToken)); + env.close(); + + // buyer creates a large enough offer. uint256 const buyOfferIndex = keylet::nftoffer(buyer, env.seq(buyer)).key; - env(token::createOffer(buyer, nftID, gwXAU(899)), + env(token::createOffer(buyer, nftID, gwXAU(1000)), token::owner(minter)); env.close(); - // broker attempts to broker the offers but cannot. + // broker charges half difference between the two offers and + // succeeds. 25% of the remaining difference goes to issuer. + // The rest goes to minter. env(token::brokerOffers( broker, buyOfferIndex, minterOfferIndex), - ter(tecINSUFFICIENT_PAYMENT)); + token::brokerFee(gwXAU(50))); env.close(); - // Cancel buyer's bad offer so the next test starts in a - // clean state. - env(token::cancelOffer(buyer, {buyOfferIndex})); + 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(1050)); + + // Burn the NFT so the next test starts with a clean state. + env(token::burn(buyer, nftID)); env.close(); } - // buyer creates a large enough offer. - uint256 const buyOfferIndex = - keylet::nftoffer(buyer, env.seq(buyer)).key; - env(token::createOffer(buyer, nftID, gwXAU(1000)), - token::owner(minter)); - env.close(); - - // Broker attempts to charge a brokerFee larger than the - // difference between the two offers but cannot. - env(token::brokerOffers(broker, buyOfferIndex, minterOfferIndex), - token::brokerFee(gwXAU(101)), - ter(tecINSUFFICIENT_PAYMENT)); - env.close(); - - // broker charges the full difference between the two offers and - // succeeds. - env(token::brokerOffers(broker, buyOfferIndex, minterOfferIndex), - token::brokerFee(gwXAU(100))); - 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(1450)); - BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(1450)); - BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(0)); - BEAST_EXPECT(env.balance(broker, gwXAU) == gwXAU(1100)); - - // Burn the NFT so the next test starts with a clean state. - env(token::burn(buyer, nftID)); - env.close(); - } - // seller offers more than buyer is asking. - // There are both transfer and broker fees, but broker takes less than - // the maximum. - { - checkOwnerCountIsOne({issuer, minter, buyer, broker}, __LINE__); - setXAUBalance_1000({issuer, minter, buyer, broker}, __LINE__); - - uint256 const nftID = mintNFT(maxTransferFee / 2); // 25% - - // minter creates their offer. - uint256 const minterOfferIndex = - keylet::nftoffer(minter, env.seq(minter)).key; - env(token::createOffer(minter, nftID, gwXAU(900)), - txflags(tfSellNFToken)); - env.close(); - - // buyer creates a large enough offer. - uint256 const buyOfferIndex = - keylet::nftoffer(buyer, env.seq(buyer)).key; - env(token::createOffer(buyer, nftID, gwXAU(1000)), - token::owner(minter)); - env.close(); - - // broker charges half difference between the two offers and - // succeeds. 25% of the remaining difference goes to issuer. - // The rest goes to minter. - env(token::brokerOffers(broker, buyOfferIndex, minterOfferIndex), - token::brokerFee(gwXAU(50))); - env.close(); + // Broker has a balance less than the seller offer + { + checkOwnerCountIsOne({issuer, minter, buyer, broker}, __LINE__); + setXAUBalance({issuer, minter, buyer}, 1000, __LINE__); + setXAUBalance({broker}, 500, __LINE__); + uint256 const nftID = mintNFT(maxTransferFee / 2); // 25% + + // minter creates their offer. + uint256 const minterOfferIndex = + keylet::nftoffer(minter, env.seq(minter)).key; + env(token::createOffer(minter, nftID, gwXAU(900)), + txflags(tfSellNFToken)); + 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(1050)); + // buyer creates a large enough offer. + uint256 const buyOfferIndex = + keylet::nftoffer(buyer, env.seq(buyer)).key; + env(token::createOffer(buyer, nftID, gwXAU(1000)), + token::owner(minter)); + env.close(); - // Burn the NFT so the next test starts with a clean state. - env(token::burn(buyer, nftID)); - env.close(); + if (tweakedFeatures[fixUnburnableNFToken]) + { + 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(); + } + } } } @@ -4823,9 +4912,14 @@ class NFToken_test : public beast::unit_test::suite Account const gw{"gw"}; IOU const gwXAU(gw["XAU"]); - // Test both with and without fixNFTokenNegOffer + // Test both with and without fixNFTokenNegOffer and + // fixUnburnableNFToken. Need to turn off fixUnburnableNFToken as well + // because that amendment came later and addressed the acceptance + // side of this issue. for (auto const& tweakedFeatures : - {features - fixNFTokenNegOffer - featureNonFungibleTokensV1_1, + {features - fixNFTokenNegOffer - featureNonFungibleTokensV1_1 - + fixUnburnableNFToken, + features - fixNFTokenNegOffer - featureNonFungibleTokensV1_1, features | fixNFTokenNegOffer}) { // There was a bug in the initial NFT implementation that @@ -4914,8 +5008,10 @@ class NFToken_test : public beast::unit_test::suite env.close(); } { - // 1. If fixNFTokenNegOffer is NOT enabled get tecSUCCESS. - // 2. If fixNFTokenNegOffer IS enabled get tecOBJECT_NOT_FOUND. + // 1. If fixNFTokenNegOffer is enabled get tecOBJECT_NOT_FOUND + // 2. If it is not enabled, but fixUnburnableNFToken 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); @@ -5047,6 +5143,757 @@ class NFToken_test : public beast::unit_test::suite } } + void + testIOUWithTransferFee(FeatureBitset features) + { + using namespace test::jtx; + + testcase("Payments with IOU transfer fees"); + + for (auto const& tweakedFeatures : + {features - fixUnburnableNFToken, features | fixUnburnableNFToken}) + { + Env env{*this, tweakedFeatures}; + + Account const minter{"minter"}; + Account const secondarySeller{"seller"}; + Account const buyer{"buyer"}; + Account const gw{"gateway"}; + Account const broker{"broker"}; + IOU const gwXAU(gw["XAU"]); + IOU const gwXPB(gw["XPB"]); + + env.fund(XRP(1000), gw, minter, secondarySeller, buyer, broker); + env.close(); + + env(trust(minter, gwXAU(2000))); + env(trust(secondarySeller, gwXAU(2000))); + env(trust(broker, gwXAU(10000))); + env(trust(buyer, gwXAU(2000))); + env(trust(buyer, gwXPB(2000))); + env.close(); + + // The IOU issuer has a 2% transfer rate + env(rate(gw, 1.02)); + env.close(); + + auto expectInitialState = [this, + &env, + &buyer, + &minter, + &secondarySeller, + &broker, + &gw, + &gwXAU, + &gwXPB]() { + // Buyer should have XAU 1000, XPB 0 + // Minter should have XAU 0, XPB 0 + // Secondary seller should have XAU 0, XPB 0 + // Broker should have XAU 5000, XPB 0 + BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(1000)); + BEAST_EXPECT(env.balance(buyer, gwXPB) == gwXPB(0)); + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(0)); + BEAST_EXPECT(env.balance(minter, gwXPB) == gwXPB(0)); + BEAST_EXPECT(env.balance(secondarySeller, gwXAU) == gwXAU(0)); + BEAST_EXPECT(env.balance(secondarySeller, gwXPB) == gwXPB(0)); + BEAST_EXPECT(env.balance(broker, gwXAU) == gwXAU(5000)); + BEAST_EXPECT(env.balance(broker, gwXPB) == gwXPB(0)); + BEAST_EXPECT(env.balance(gw, buyer["XAU"]) == gwXAU(-1000)); + BEAST_EXPECT(env.balance(gw, buyer["XPB"]) == gwXPB(0)); + BEAST_EXPECT(env.balance(gw, minter["XAU"]) == gwXAU(0)); + BEAST_EXPECT(env.balance(gw, minter["XPB"]) == gwXPB(0)); + BEAST_EXPECT( + env.balance(gw, secondarySeller["XAU"]) == gwXAU(0)); + BEAST_EXPECT( + env.balance(gw, secondarySeller["XPB"]) == gwXPB(0)); + BEAST_EXPECT(env.balance(gw, broker["XAU"]) == gwXAU(-5000)); + BEAST_EXPECT(env.balance(gw, broker["XPB"]) == gwXPB(0)); + }; + + auto reinitializeTrustLineBalances = [&expectInitialState, + &env, + &buyer, + &minter, + &secondarySeller, + &broker, + &gw, + &gwXAU, + &gwXPB]() { + if (auto const difference = + gwXAU(1000) - env.balance(buyer, gwXAU); + difference > gwXAU(0)) + env(pay(gw, buyer, difference)); + if (env.balance(buyer, gwXPB) > gwXPB(0)) + env(pay(buyer, gw, env.balance(buyer, gwXPB))); + if (env.balance(minter, gwXAU) > gwXAU(0)) + env(pay(minter, gw, env.balance(minter, gwXAU))); + if (env.balance(minter, gwXPB) > gwXPB(0)) + env(pay(minter, gw, env.balance(minter, gwXPB))); + if (env.balance(secondarySeller, gwXAU) > gwXAU(0)) + env( + pay(secondarySeller, + gw, + env.balance(secondarySeller, gwXAU))); + if (env.balance(secondarySeller, gwXPB) > gwXPB(0)) + env( + pay(secondarySeller, + gw, + env.balance(secondarySeller, gwXPB))); + auto brokerDiff = gwXAU(5000) - env.balance(broker, gwXAU); + if (brokerDiff > gwXAU(0)) + env(pay(gw, broker, brokerDiff)); + else if (brokerDiff < gwXAU(0)) + { + brokerDiff.negate(); + env(pay(broker, gw, brokerDiff)); + } + if (env.balance(broker, gwXPB) > gwXPB(0)) + env(pay(broker, gw, env.balance(broker, gwXPB))); + env.close(); + expectInitialState(); + }; + + auto mintNFT = [&env](Account const& minter, int transferFee = 0) { + uint256 const nftID = token::getNextID( + env, minter, 0, tfTransferable, transferFee); + env(token::mint(minter), + token::xferFee(transferFee), + txflags(tfTransferable)); + env.close(); + return nftID; + }; + + auto createBuyOffer = + [&env]( + Account const& offerer, + Account const& owner, + uint256 const& nftID, + STAmount const& amount, + std::optional const terCode = {}) { + uint256 const offerID = + keylet::nftoffer(offerer, env.seq(offerer)).key; + env(token::createOffer(offerer, nftID, amount), + token::owner(owner), + terCode ? ter(*terCode) + : ter(static_cast(tesSUCCESS))); + env.close(); + return offerID; + }; + + auto createSellOffer = + [&env]( + Account const& offerer, + uint256 const& nftID, + STAmount const& amount, + std::optional const terCode = {}) { + uint256 const offerID = + keylet::nftoffer(offerer, env.seq(offerer)).key; + env(token::createOffer(offerer, nftID, amount), + txflags(tfSellNFToken), + terCode ? ter(*terCode) + : ter(static_cast(tesSUCCESS))); + env.close(); + return offerID; + }; + + { + // Buyer attempts to send 100% of their balance of an IOU + // (sellside) + reinitializeTrustLineBalances(); + auto const nftID = mintNFT(minter); + auto const offerID = + createSellOffer(minter, nftID, gwXAU(1000)); + auto const sellTER = tweakedFeatures[fixUnburnableNFToken] + ? static_cast(tecINSUFFICIENT_FUNDS) + : static_cast(tesSUCCESS); + env(token::acceptSellOffer(buyer, offerID), ter(sellTER)); + env.close(); + + if (tweakedFeatures[fixUnburnableNFToken]) + 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)); + } + } + { + // Buyer attempts to send 100% of their balance of an IOU + // (buyside) + reinitializeTrustLineBalances(); + auto const nftID = mintNFT(minter); + auto const offerID = + createBuyOffer(buyer, minter, nftID, gwXAU(1000)); + auto const sellTER = tweakedFeatures[fixUnburnableNFToken] + ? static_cast(tecINSUFFICIENT_FUNDS) + : static_cast(tesSUCCESS); + env(token::acceptBuyOffer(minter, offerID), ter(sellTER)); + env.close(); + + if (tweakedFeatures[fixUnburnableNFToken]) + 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)); + } + } + { + // Buyer attempts to send an amount less than 100% of their + // balance of an IOU, but such that the addition of the transfer + // fee would be greater than the buyer's balance (sellside) + reinitializeTrustLineBalances(); + auto const nftID = mintNFT(minter); + auto const offerID = createSellOffer(minter, nftID, gwXAU(995)); + auto const sellTER = tweakedFeatures[fixUnburnableNFToken] + ? static_cast(tecINSUFFICIENT_FUNDS) + : static_cast(tesSUCCESS); + env(token::acceptSellOffer(buyer, offerID), ter(sellTER)); + env.close(); + + if (tweakedFeatures[fixUnburnableNFToken]) + 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)); + } + } + { + // Buyer attempts to send an amount less than 100% of their + // balance of an IOU, but such that the addition of the transfer + // fee would be greater than the buyer's balance (buyside) + reinitializeTrustLineBalances(); + auto const nftID = mintNFT(minter); + auto const offerID = + createBuyOffer(buyer, minter, nftID, gwXAU(995)); + auto const sellTER = tweakedFeatures[fixUnburnableNFToken] + ? static_cast(tecINSUFFICIENT_FUNDS) + : static_cast(tesSUCCESS); + env(token::acceptBuyOffer(minter, offerID), ter(sellTER)); + env.close(); + + if (tweakedFeatures[fixUnburnableNFToken]) + 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)); + } + } + { + // Buyer attempts to send an amount less than 100% of their + // balance of an IOU with a transfer fee, and such that the + // addition of the transfer fee is still less than their balance + // (sellside) + reinitializeTrustLineBalances(); + auto const nftID = mintNFT(minter); + auto const offerID = createSellOffer(minter, nftID, gwXAU(900)); + env(token::acceptSellOffer(buyer, offerID)); + env.close(); + + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(900)); + BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(82)); + BEAST_EXPECT(env.balance(gw, minter["XAU"]) == gwXAU(-900)); + BEAST_EXPECT(env.balance(gw, buyer["XAU"]) == gwXAU(-82)); + } + { + // Buyer attempts to send an amount less than 100% of their + // balance of an IOU with a transfer fee, and such that the + // addition of the transfer fee is still less than their balance + // (buyside) + reinitializeTrustLineBalances(); + auto const nftID = mintNFT(minter); + auto const offerID = + createBuyOffer(buyer, minter, nftID, gwXAU(900)); + env(token::acceptBuyOffer(minter, offerID)); + env.close(); + + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(900)); + BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(82)); + BEAST_EXPECT(env.balance(gw, minter["XAU"]) == gwXAU(-900)); + BEAST_EXPECT(env.balance(gw, buyer["XAU"]) == gwXAU(-82)); + } + { + // Buyer attempts to send an amount less than 100% of their + // balance of an IOU with a transfer fee, and such that the + // addition of the transfer fee is equal than their balance + // (sellside) + reinitializeTrustLineBalances(); + + // pay them an additional XAU 20 to cover transfer rate + env(pay(gw, buyer, gwXAU(20))); + env.close(); + + auto const nftID = mintNFT(minter); + auto const offerID = + createSellOffer(minter, nftID, gwXAU(1000)); + env(token::acceptSellOffer(buyer, offerID)); + env.close(); + + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(1000)); + BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(0)); + BEAST_EXPECT(env.balance(gw, minter["XAU"]) == gwXAU(-1000)); + BEAST_EXPECT(env.balance(gw, buyer["XAU"]) == gwXAU(0)); + } + { + // Buyer attempts to send an amount less than 100% of their + // balance of an IOU with a transfer fee, and such that the + // addition of the transfer fee is equal than their balance + // (buyside) + reinitializeTrustLineBalances(); + + // pay them an additional XAU 20 to cover transfer rate + env(pay(gw, buyer, gwXAU(20))); + env.close(); + + auto const nftID = mintNFT(minter); + auto const offerID = + createBuyOffer(buyer, minter, nftID, gwXAU(1000)); + env(token::acceptBuyOffer(minter, offerID)); + env.close(); + + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(1000)); + BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(0)); + BEAST_EXPECT(env.balance(gw, minter["XAU"]) == gwXAU(-1000)); + BEAST_EXPECT(env.balance(gw, buyer["XAU"]) == gwXAU(0)); + } + { + // Gateway attempts to buy NFT with their own IOU - no + // transfer fee is calculated here (sellside) + reinitializeTrustLineBalances(); + + auto const nftID = mintNFT(minter); + auto const offerID = + createSellOffer(minter, nftID, gwXAU(1000)); + auto const sellTER = tweakedFeatures[fixUnburnableNFToken] + ? static_cast(tesSUCCESS) + : static_cast(tecINSUFFICIENT_FUNDS); + env(token::acceptSellOffer(gw, offerID), ter(sellTER)); + env.close(); + + if (tweakedFeatures[fixUnburnableNFToken]) + { + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(1000)); + BEAST_EXPECT( + env.balance(gw, minter["XAU"]) == gwXAU(-1000)); + } + else + expectInitialState(); + } + { + // Gateway attempts to buy NFT with their own IOU - no + // transfer fee is calculated here (buyside) + reinitializeTrustLineBalances(); + + auto const nftID = mintNFT(minter); + auto const offerTER = tweakedFeatures[fixUnburnableNFToken] + ? static_cast(tesSUCCESS) + : static_cast(tecUNFUNDED_OFFER); + auto const offerID = + createBuyOffer(gw, minter, nftID, gwXAU(1000), {offerTER}); + auto const sellTER = tweakedFeatures[fixUnburnableNFToken] + ? static_cast(tesSUCCESS) + : static_cast(tecOBJECT_NOT_FOUND); + env(token::acceptBuyOffer(minter, offerID), ter(sellTER)); + env.close(); + + if (tweakedFeatures[fixUnburnableNFToken]) + { + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(1000)); + BEAST_EXPECT( + env.balance(gw, minter["XAU"]) == gwXAU(-1000)); + } + else + expectInitialState(); + } + { + // Gateway attempts to buy NFT with their own IOU for more + // than minter trusts (sellside) + reinitializeTrustLineBalances(); + auto const nftID = mintNFT(minter); + auto const offerID = + createSellOffer(minter, nftID, gwXAU(5000)); + auto const sellTER = tweakedFeatures[fixUnburnableNFToken] + ? static_cast(tesSUCCESS) + : static_cast(tecINSUFFICIENT_FUNDS); + env(token::acceptSellOffer(gw, offerID), ter(sellTER)); + env.close(); + + if (tweakedFeatures[fixUnburnableNFToken]) + { + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(5000)); + BEAST_EXPECT( + env.balance(gw, minter["XAU"]) == gwXAU(-5000)); + } + else + expectInitialState(); + } + { + // Gateway attempts to buy NFT with their own IOU for more + // than minter trusts (buyside) + reinitializeTrustLineBalances(); + + auto const nftID = mintNFT(minter); + auto const offerTER = tweakedFeatures[fixUnburnableNFToken] + ? static_cast(tesSUCCESS) + : static_cast(tecUNFUNDED_OFFER); + auto const offerID = + createBuyOffer(gw, minter, nftID, gwXAU(5000), {offerTER}); + auto const sellTER = tweakedFeatures[fixUnburnableNFToken] + ? static_cast(tesSUCCESS) + : static_cast(tecOBJECT_NOT_FOUND); + env(token::acceptBuyOffer(minter, offerID), ter(sellTER)); + env.close(); + + if (tweakedFeatures[fixUnburnableNFToken]) + { + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(5000)); + BEAST_EXPECT( + env.balance(gw, minter["XAU"]) == gwXAU(-5000)); + } + else + expectInitialState(); + } + { + // Gateway is the NFT minter and attempts to sell NFT for an + // amount that would be greater than a balance if there were a + // transfer fee calculated in this transaction. (sellside) + reinitializeTrustLineBalances(); + auto const nftID = mintNFT(gw); + auto const offerID = createSellOffer(gw, nftID, gwXAU(1000)); + env(token::acceptSellOffer(buyer, offerID)); + env.close(); + + BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(0)); + BEAST_EXPECT(env.balance(gw, buyer["XAU"]) == gwXAU(0)); + } + { + // Gateway is the NFT minter and attempts to sell NFT for an + // amount that would be greater than a balance if there were a + // transfer fee calculated in this transaction. (buyside) + reinitializeTrustLineBalances(); + + auto const nftID = mintNFT(gw); + auto const offerID = + createBuyOffer(buyer, gw, nftID, gwXAU(1000)); + env(token::acceptBuyOffer(gw, offerID)); + env.close(); + + BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(0)); + BEAST_EXPECT(env.balance(gw, buyer["XAU"]) == gwXAU(0)); + } + { + // Gateway is the NFT minter and attempts to sell NFT for an + // amount that is greater than a balance before transfer fees. + // (sellside) + reinitializeTrustLineBalances(); + auto const nftID = mintNFT(gw); + auto const offerID = createSellOffer(gw, nftID, gwXAU(2000)); + env(token::acceptSellOffer(buyer, offerID), + ter(static_cast(tecINSUFFICIENT_FUNDS))); + env.close(); + expectInitialState(); + } + { + // Gateway is the NFT minter and attempts to sell NFT for an + // amount that is greater than a balance before transfer fees. + // (buyside) + reinitializeTrustLineBalances(); + auto const nftID = mintNFT(gw); + auto const offerID = + createBuyOffer(buyer, gw, nftID, gwXAU(2000)); + env(token::acceptBuyOffer(gw, offerID), + ter(static_cast(tecINSUFFICIENT_FUNDS))); + env.close(); + expectInitialState(); + } + { + // Minter attempts to sell the token for XPB 10, which they + // have no trust line for and buyer has none of (sellside). + reinitializeTrustLineBalances(); + auto const nftID = mintNFT(minter); + auto const offerID = createSellOffer(minter, nftID, gwXPB(10)); + env(token::acceptSellOffer(buyer, offerID), + ter(static_cast(tecINSUFFICIENT_FUNDS))); + env.close(); + expectInitialState(); + } + { + // Minter attempts to sell the token for XPB 10, which they + // have no trust line for and buyer has none of (buyside). + reinitializeTrustLineBalances(); + auto const nftID = mintNFT(minter); + auto const offerID = createBuyOffer( + buyer, + minter, + nftID, + gwXPB(10), + {static_cast(tecUNFUNDED_OFFER)}); + env(token::acceptBuyOffer(minter, offerID), + ter(static_cast(tecOBJECT_NOT_FOUND))); + env.close(); + expectInitialState(); + } + { + // Minter attempts to sell the token for XPB 10 and the buyer + // has it but the minter has no trust line. Trust line is + // created as a result of the tx (sellside). + reinitializeTrustLineBalances(); + env(pay(gw, buyer, gwXPB(100))); + env.close(); + + auto const nftID = mintNFT(minter); + auto const offerID = createSellOffer(minter, nftID, gwXPB(10)); + env(token::acceptSellOffer(buyer, offerID)); + env.close(); + + BEAST_EXPECT(env.balance(minter, gwXPB) == gwXPB(10)); + BEAST_EXPECT(env.balance(buyer, gwXPB) == gwXPB(89.8)); + BEAST_EXPECT(env.balance(gw, minter["XPB"]) == gwXPB(-10)); + BEAST_EXPECT(env.balance(gw, buyer["XPB"]) == gwXPB(-89.8)); + } + { + // Minter attempts to sell the token for XPB 10 and the buyer + // has it but the minter has no trust line. Trust line is + // created as a result of the tx (buyside). + reinitializeTrustLineBalances(); + env(pay(gw, buyer, gwXPB(100))); + env.close(); + + auto const nftID = mintNFT(minter); + auto const offerID = + createBuyOffer(buyer, minter, nftID, gwXPB(10)); + env(token::acceptBuyOffer(minter, offerID)); + env.close(); + + BEAST_EXPECT(env.balance(minter, gwXPB) == gwXPB(10)); + BEAST_EXPECT(env.balance(buyer, gwXPB) == gwXPB(89.8)); + BEAST_EXPECT(env.balance(gw, minter["XPB"]) == gwXPB(-10)); + BEAST_EXPECT(env.balance(gw, buyer["XPB"]) == gwXPB(-89.8)); + } + { + // There is a transfer fee on the NFT and buyer has exact + // amount (sellside) + reinitializeTrustLineBalances(); + + // secondarySeller has to sell it because transfer fees only + // happen on secondary sales + auto const nftID = mintNFT(minter, 3000); // 3% + auto const primaryOfferID = + createSellOffer(minter, nftID, XRP(0)); + env(token::acceptSellOffer(secondarySeller, primaryOfferID)); + env.close(); + + // now we can do a secondary sale + auto const offerID = + createSellOffer(secondarySeller, nftID, gwXAU(1000)); + auto const sellTER = tweakedFeatures[fixUnburnableNFToken] + ? static_cast(tecINSUFFICIENT_FUNDS) + : static_cast(tesSUCCESS); + env(token::acceptSellOffer(buyer, offerID), ter(sellTER)); + env.close(); + + if (tweakedFeatures[fixUnburnableNFToken]) + 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)); + } + } + { + // There is a transfer fee on the NFT and buyer has exact + // amount (buyside) + reinitializeTrustLineBalances(); + + // secondarySeller has to sell it because transfer fees only + // happen on secondary sales + auto const nftID = mintNFT(minter, 3000); // 3% + auto const primaryOfferID = + createSellOffer(minter, nftID, XRP(0)); + env(token::acceptSellOffer(secondarySeller, primaryOfferID)); + env.close(); + + // now we can do a secondary sale + auto const offerID = + createBuyOffer(buyer, secondarySeller, nftID, gwXAU(1000)); + auto const sellTER = tweakedFeatures[fixUnburnableNFToken] + ? static_cast(tecINSUFFICIENT_FUNDS) + : static_cast(tesSUCCESS); + env(token::acceptBuyOffer(secondarySeller, offerID), + ter(sellTER)); + env.close(); + + if (tweakedFeatures[fixUnburnableNFToken]) + 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)); + } + } + { + // There is a transfer fee on the NFT and buyer has enough + // (sellside) + reinitializeTrustLineBalances(); + + // secondarySeller has to sell it because transfer fees only + // happen on secondary sales + auto const nftID = mintNFT(minter, 3000); // 3% + auto const primaryOfferID = + createSellOffer(minter, nftID, XRP(0)); + env(token::acceptSellOffer(secondarySeller, primaryOfferID)); + env.close(); + + // now we can do a secondary sale + auto const offerID = + createSellOffer(secondarySeller, nftID, gwXAU(900)); + env(token::acceptSellOffer(buyer, offerID)); + env.close(); + + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(27)); + BEAST_EXPECT(env.balance(secondarySeller, gwXAU) == gwXAU(873)); + BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(82)); + BEAST_EXPECT(env.balance(gw, minter["XAU"]) == gwXAU(-27)); + BEAST_EXPECT( + env.balance(gw, secondarySeller["XAU"]) == gwXAU(-873)); + BEAST_EXPECT(env.balance(gw, buyer["XAU"]) == gwXAU(-82)); + } + { + // There is a transfer fee on the NFT and buyer has enough + // (buyside) + reinitializeTrustLineBalances(); + + // secondarySeller has to sell it because transfer fees only + // happen on secondary sales + auto const nftID = mintNFT(minter, 3000); // 3% + auto const primaryOfferID = + createSellOffer(minter, nftID, XRP(0)); + env(token::acceptSellOffer(secondarySeller, primaryOfferID)); + env.close(); + + // now we can do a secondary sale + auto const offerID = + createBuyOffer(buyer, secondarySeller, nftID, gwXAU(900)); + env(token::acceptBuyOffer(secondarySeller, offerID)); + env.close(); + + // receives 3% of 900 - 27 + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(27)); + // receives 97% of 900 - 873 + BEAST_EXPECT(env.balance(secondarySeller, gwXAU) == gwXAU(873)); + // pays 900 plus 2% transfer fee on XAU - 918 + BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(82)); + BEAST_EXPECT(env.balance(gw, minter["XAU"]) == gwXAU(-27)); + BEAST_EXPECT( + env.balance(gw, secondarySeller["XAU"]) == gwXAU(-873)); + BEAST_EXPECT(env.balance(gw, buyer["XAU"]) == gwXAU(-82)); + } + { + // There is a broker fee on the NFT. XAU transfer fee is only + // calculated from the buyer's output, not deducted from + // broker fee. + // + // For a payment of 500 with a 2% IOU transfee fee and 100 + // broker fee: + // + // A) Total sale amount + IOU transfer fee is paid by buyer + // (Buyer pays (1.02 * 500) = 510) + // B) GW receives the additional IOU transfer fee + // (GW receives 10 from buyer calculated above) + // C) Broker receives broker fee (no IOU transfer fee) + // (Broker receives 100 from buyer) + // D) Seller receives balance (no IOU transfer fee) + // (Seller receives (510 - 10 - 100) = 400) + reinitializeTrustLineBalances(); + + auto const nftID = mintNFT(minter); + auto const sellOffer = + createSellOffer(minter, nftID, gwXAU(300)); + auto const buyOffer = + createBuyOffer(buyer, minter, nftID, gwXAU(500)); + env(token::brokerOffers(broker, buyOffer, sellOffer), + token::brokerFee(gwXAU(100))); + env.close(); + + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(400)); + BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(490)); + BEAST_EXPECT(env.balance(broker, gwXAU) == gwXAU(5100)); + BEAST_EXPECT(env.balance(gw, minter["XAU"]) == gwXAU(-400)); + BEAST_EXPECT(env.balance(gw, buyer["XAU"]) == gwXAU(-490)); + BEAST_EXPECT(env.balance(gw, broker["XAU"]) == gwXAU(-5100)); + } + { + // There is broker and transfer fee on the NFT + // + // For a payment of 500 with a 2% IOU transfer fee, 3% NFT + // transfer fee, and 100 broker fee: + // + // A) Total sale amount + IOU transfer fee is paid by buyer + // (Buyer pays (1.02 * 500) = 510) + // B) GW receives the additional IOU transfer fee + // (GW receives 10 from buyer calculated above) + // C) Broker receives broker fee (no IOU transfer fee) + // (Broker receives 100 from buyer) + // D) Minter receives transfer fee (no IOU transfer fee) + // (Minter receives 0.03 * (510 - 10 - 100) = 12) + // E) Seller receives balance (no IOU transfer fee) + // (Seller receives (510 - 10 - 100 - 12) = 388) + reinitializeTrustLineBalances(); + + // secondarySeller has to sell it because transfer fees only + // happen on secondary sales + auto const nftID = mintNFT(minter, 3000); // 3% + auto const primaryOfferID = + createSellOffer(minter, nftID, XRP(0)); + env(token::acceptSellOffer(secondarySeller, primaryOfferID)); + env.close(); + + // now we can do a secondary sale + auto const sellOffer = + createSellOffer(secondarySeller, nftID, gwXAU(300)); + auto const buyOffer = + createBuyOffer(buyer, secondarySeller, nftID, gwXAU(500)); + env(token::brokerOffers(broker, buyOffer, sellOffer), + token::brokerFee(gwXAU(100))); + env.close(); + + BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(12)); + BEAST_EXPECT(env.balance(buyer, gwXAU) == gwXAU(490)); + BEAST_EXPECT(env.balance(secondarySeller, gwXAU) == gwXAU(388)); + BEAST_EXPECT(env.balance(broker, gwXAU) == gwXAU(5100)); + BEAST_EXPECT(env.balance(gw, minter["XAU"]) == gwXAU(-12)); + BEAST_EXPECT(env.balance(gw, buyer["XAU"]) == gwXAU(-490)); + BEAST_EXPECT( + env.balance(gw, secondarySeller["XAU"]) == gwXAU(-388)); + BEAST_EXPECT(env.balance(gw, broker["XAU"]) == gwXAU(-5100)); + } + } + } + void testWithFeats(FeatureBitset features) { @@ -5076,6 +5923,7 @@ class NFToken_test : public beast::unit_test::suite testNFTokenDeleteAccount(features); testNftXxxOffers(features); testFixNFTokenNegOffer(features); + testIOUWithTransferFee(features); } public: @@ -5086,6 +5934,8 @@ class NFToken_test : public beast::unit_test::suite FeatureBitset const all{supported_amendments()}; FeatureBitset const fixNFTDir{fixNFTokenDirV1}; + // TODO too many tests are being run - ths fixNFTDir check should be + // pushed into the tests that use it testWithFeats(all - fixNFTDir); testWithFeats(all - disallowIncoming); testWithFeats(all); From 39c32561bdd47e4c7feeaffe4c71f02c2fd8b5c0 Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Thu, 9 Feb 2023 21:15:22 -0800 Subject: [PATCH 4/6] Prevent brokered sale of NFToken to owner: (#4403) Fixes #4374 It was possible for a broker to combine a sell and a buy offer from an account that already owns an NFT. Such brokering extracts money from the NFT owner and provides no benefit in return. With this amendment, the code detects when a broker is returning an NFToken to its initial owner and prohibits the transaction. This forbids a broker from selling an NFToken to the account that already owns the token. This fixes a bug in the original implementation of XLS-20. Thanks to @nixer89 for suggesting this fix. --- src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp | 6 + src/test/app/NFToken_test.cpp | 119 +++++++++++++++++- 2 files changed, 120 insertions(+), 5 deletions(-) diff --git a/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp b/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp index c335f8d28fd..257bda5c051 100644 --- a/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp +++ b/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp @@ -107,6 +107,12 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx) if ((*bo)[sfAmount].issue() != (*so)[sfAmount].issue()) return tecNFTOKEN_BUY_SELL_MISMATCH; + // 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(fixUnburnableNFToken) && + ((*bo)[sfOwner] == (*so)[sfOwner])) + return tecCANT_ACCEPT_OWN_NFTOKEN_OFFER; + // Ensure that the buyer is willing to pay at least as much as the // seller is requesting: if ((*so)[sfAmount] > (*bo)[sfAmount]) diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 33d725e5a17..0c428e6fac9 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -4613,7 +4613,7 @@ class NFToken_test : public beast::unit_test::suite txflags(tfTransferable)); env.close(); - // At the momement issuer and minter cannot delete themselves. + // At the moment issuer and minter cannot delete themselves. // o issuer has an issued NFT in the ledger. // o minter owns an NFT. env(acctdelete(issuer, daria), fee(XRP(50)), ter(tecHAS_OBLIGATIONS)); @@ -5894,6 +5894,115 @@ class NFToken_test : public beast::unit_test::suite } } + void + testBrokeredSaleToSelf(FeatureBitset features) + { + // There was a bug that if an account had... + // + // 1. An NFToken, and + // 2. An offer on the ledger to buy that same token, and + // 3. Also an offer of the ledger to sell that same token, + // + // Then someone could broker the two offers. This would result in + // the NFToken being bought and returned to the original owner and + // the broker pocketing the profit. + // + // This unit test verifies that the fixUnburnableNFToken amendment + // fixes that bug. + testcase("Brokered sale to self"); + + using namespace test::jtx; + + Account const alice{"alice"}; + Account const bob{"bob"}; + Account const broker{"broker"}; + + Env env{*this, features}; + env.fund(XRP(10000), alice, bob, broker); + env.close(); + + // For this scenario to occur we need the following steps: + // + // 1. alice mints NFT. + // 2. bob creates a buy offer for it for 5 XRP. + // 3. alice decides to gift the NFT to bob for 0. + // creating a sell offer (hopefully using a destination too) + // 4. Bob accepts the sell offer, because it is better than + // paying 5 XRP. + // 5. At this point, bob has the NFT and still has their buy + // offer from when they did not have the NFT! This is because + // the order book is not cleared when an NFT changes hands. + // 6. Now that Bob owns the NFT, he cannot create new buy offers. + // However he still has one left over from when he did not own + // it. He can create new sell offers and does. + // 7. Now that bob has both a buy and a sell offer for the same NFT, + // a broker can sell the NFT that bob owns to bob and pocket the + // difference. + uint256 const nftId{token::getNextID(env, alice, 0u, tfTransferable)}; + env(token::mint(alice, 0u), txflags(tfTransferable)); + env.close(); + + // Bob creates a buy offer for 5 XRP. Alice creates a sell offer + // for 0 XRP. + uint256 const bobBuyOfferIndex = + keylet::nftoffer(bob, env.seq(bob)).key; + env(token::createOffer(bob, nftId, XRP(5)), token::owner(alice)); + + uint256 const aliceSellOfferIndex = + keylet::nftoffer(alice, env.seq(alice)).key; + env(token::createOffer(alice, nftId, XRP(0)), + token::destination(bob), + txflags(tfSellNFToken)); + env.close(); + + // bob accepts alice's offer but forgets to remove the old buy offer. + env(token::acceptSellOffer(bob, aliceSellOfferIndex)); + env.close(); + + // Note that bob still has a buy offer on the books. + BEAST_EXPECT(env.le(keylet::nftoffer(bobBuyOfferIndex))); + + // Bob creates a sell offer for the gift NFT from alice. + uint256 const bobSellOfferIndex = + keylet::nftoffer(bob, env.seq(bob)).key; + env(token::createOffer(bob, nftId, XRP(4)), txflags(tfSellNFToken)); + env.close(); + + // bob now has a buy offer and a sell offer on the books. A broker + // spots this and swoops in to make a profit. + BEAST_EXPECT(nftCount(env, bob) == 1); + auto const bobsPriorBalance = env.balance(bob); + auto const brokersPriorBalance = env.balance(broker); + TER expectTer = features[fixUnburnableNFToken] + ? TER(tecCANT_ACCEPT_OWN_NFTOKEN_OFFER) + : TER(tesSUCCESS); + env(token::brokerOffers(broker, bobBuyOfferIndex, bobSellOfferIndex), + token::brokerFee(XRP(1)), + ter(expectTer)); + 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) - drops(10)); + } + 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 - drops(10)); + } + } + void testWithFeats(FeatureBitset features) { @@ -5924,6 +6033,7 @@ class NFToken_test : public beast::unit_test::suite testNftXxxOffers(features); testFixNFTokenNegOffer(features); testIOUWithTransferFee(features); + testBrokeredSaleToSelf(features); } public: @@ -5934,10 +6044,9 @@ class NFToken_test : public beast::unit_test::suite FeatureBitset const all{supported_amendments()}; FeatureBitset const fixNFTDir{fixNFTokenDirV1}; - // TODO too many tests are being run - ths fixNFTDir check should be - // pushed into the tests that use it - testWithFeats(all - fixNFTDir); - testWithFeats(all - disallowIncoming); + testWithFeats(all - fixNFTDir - fixUnburnableNFToken); + testWithFeats(all - disallowIncoming - fixUnburnableNFToken); + testWithFeats(all - fixUnburnableNFToken); testWithFeats(all); } }; From b72a87c7d3629c19e37b5f5fc86232e60540a80f Mon Sep 17 00:00:00 2001 From: Denis Angell Date: Mon, 13 Feb 2023 13:02:24 -0500 Subject: [PATCH 5/6] Only account specified as destination can settle through brokerage: (#4399) Without this amendment, for NFTs using broker mode, if the sell offer contains a destination and that destination is the buyer account, anyone can broker the transaction. Also, if a buy offer contains a destination and that destination is the seller account, anyone can broker the transaction. This is not ideal and is misleading. Instead, with this amendment: If you set a destination, that destination needs to be the account settling the transaction. So, the broker must be the destination if they want to settle. If the buyer is the destination, then the buyer must accept the sell offer, as you cannot broker your own offers. If users want their offers open to the public, then they should not set a destination. On the other hand, if users want to limit who can settle the offers, then they would set a destination. Unit tests: 1. The broker cannot broker a destination offer to the buyer and the buyer must accept the sell offer. (0 transfer) 2. If the broker is the destination, the broker will take the difference. (broker mode) --- src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp | 36 ++++-- src/test/app/NFToken_test.cpp | 113 +++++++++++------- 2 files changed, 101 insertions(+), 48 deletions(-) diff --git a/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp b/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp index 257bda5c051..c420bfc6197 100644 --- a/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp +++ b/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp @@ -118,20 +118,40 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx) if ((*so)[sfAmount] > (*bo)[sfAmount]) return tecINSUFFICIENT_PAYMENT; - // If the buyer specified a destination, that destination must be - // the seller or the broker. + // If the buyer specified a destination if (auto const dest = bo->at(~sfDestination)) { - if (*dest != so->at(sfOwner) && *dest != ctx.tx[sfAccount]) - return tecNFTOKEN_BUY_SELL_MISMATCH; + // fixUnburnableNFToken + if (ctx.view.rules().enabled(fixUnburnableNFToken)) + { + // the destination may only be the account brokering the offer + if (*dest != ctx.tx[sfAccount]) + return tecNO_PERMISSION; + } + else + { + // the destination must be the seller or the broker. + if (*dest != so->at(sfOwner) && *dest != ctx.tx[sfAccount]) + return tecNFTOKEN_BUY_SELL_MISMATCH; + } } - // If the seller specified a destination, that destination must be - // the buyer or the broker. + // If the seller specified a destination if (auto const dest = so->at(~sfDestination)) { - if (*dest != bo->at(sfOwner) && *dest != ctx.tx[sfAccount]) - return tecNFTOKEN_BUY_SELL_MISMATCH; + // fixUnburnableNFToken + if (ctx.view.rules().enabled(fixUnburnableNFToken)) + { + // the destination may only be the account brokering the offer + if (*dest != ctx.tx[sfAccount]) + return tecNO_PERMISSION; + } + else + { + // the destination must be the buyer or the broker. + if (*dest != bo->at(sfOwner) && *dest != ctx.tx[sfAccount]) + return tecNFTOKEN_BUY_SELL_MISMATCH; + } } // The broker can specify an amount that represents their cut; if they diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 0c428e6fac9..d581a6d0d90 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -2878,15 +2878,20 @@ class NFToken_test : public beast::unit_test::suite BEAST_EXPECT(ownerCount(env, minter) == 2); BEAST_EXPECT(ownerCount(env, buyer) == 1); - // issuer cannot broker the offers, because they are not the - // Destination. - env(token::brokerOffers( - issuer, offerBuyerToMinter, offerMinterToBroker), - ter(tecNFTOKEN_BUY_SELL_MISMATCH)); - env.close(); - BEAST_EXPECT(ownerCount(env, issuer) == 0); - BEAST_EXPECT(ownerCount(env, minter) == 2); - BEAST_EXPECT(ownerCount(env, buyer) == 1); + { + // issuer cannot broker the offers, because they are not the + // Destination. + TER const expectTer = features[fixUnburnableNFToken] + ? tecNO_PERMISSION + : tecNFTOKEN_BUY_SELL_MISMATCH; + env(token::brokerOffers( + issuer, offerBuyerToMinter, offerMinterToBroker), + ter(expectTer)); + env.close(); + BEAST_EXPECT(ownerCount(env, issuer) == 0); + BEAST_EXPECT(ownerCount(env, minter) == 2); + BEAST_EXPECT(ownerCount(env, buyer) == 1); + } // Since broker is the sell offer's destination, they can broker // the two offers. @@ -2923,29 +2928,52 @@ class NFToken_test : public beast::unit_test::suite BEAST_EXPECT(ownerCount(env, minter) == 1); BEAST_EXPECT(ownerCount(env, buyer) == 2); - // Cannot broker offers when the sell destination is not the buyer. - env(token::brokerOffers( - broker, offerIssuerToBuyer, offerBuyerToMinter), - ter(tecNFTOKEN_BUY_SELL_MISMATCH)); - env.close(); - BEAST_EXPECT(ownerCount(env, issuer) == 1); - BEAST_EXPECT(ownerCount(env, minter) == 1); - BEAST_EXPECT(ownerCount(env, buyer) == 2); + { + // Cannot broker offers when the sell destination is not the + // buyer. + TER const expectTer = features[fixUnburnableNFToken] + ? tecNO_PERMISSION + : tecNFTOKEN_BUY_SELL_MISMATCH; + env(token::brokerOffers( + broker, offerIssuerToBuyer, offerBuyerToMinter), + ter(expectTer)); + env.close(); - // Broker is successful when destination is buyer. - env(token::brokerOffers( - broker, offerMinterToBuyer, offerBuyerToMinter)); - env.close(); - BEAST_EXPECT(ownerCount(env, issuer) == 1); - BEAST_EXPECT(ownerCount(env, minter) == 1); - BEAST_EXPECT(ownerCount(env, buyer) == 0); + BEAST_EXPECT(ownerCount(env, issuer) == 1); + BEAST_EXPECT(ownerCount(env, minter) == 1); + BEAST_EXPECT(ownerCount(env, buyer) == 2); - // Clean out the unconsumed offer. - env(token::cancelOffer(issuer, {offerIssuerToBuyer})); - env.close(); - BEAST_EXPECT(ownerCount(env, issuer) == 0); - BEAST_EXPECT(ownerCount(env, minter) == 1); - BEAST_EXPECT(ownerCount(env, buyer) == 0); + // amendment switch: When enabled the broker fails, when + // disabled the broker succeeds if the destination is the buyer. + TER const eexpectTer = features[fixUnburnableNFToken] + ? tecNO_PERMISSION + : TER(tesSUCCESS); + env(token::brokerOffers( + broker, offerMinterToBuyer, offerBuyerToMinter), + ter(eexpectTer)); + env.close(); + + if (features[fixUnburnableNFToken]) + // Buyer is successful with acceptOffer. + env(token::acceptBuyOffer(buyer, offerMinterToBuyer)); + env.close(); + + // Clean out the unconsumed offer. + env(token::cancelOffer(buyer, {offerBuyerToMinter})); + env.close(); + + BEAST_EXPECT(ownerCount(env, issuer) == 1); + BEAST_EXPECT(ownerCount(env, minter) == 1); + BEAST_EXPECT(ownerCount(env, buyer) == 0); + + // Clean out the unconsumed offer. + env(token::cancelOffer(issuer, {offerIssuerToBuyer})); + env.close(); + BEAST_EXPECT(ownerCount(env, issuer) == 0); + BEAST_EXPECT(ownerCount(env, minter) == 1); + BEAST_EXPECT(ownerCount(env, buyer) == 0); + return; + } } // Show that if a buy and a sell offer both have the same destination, @@ -2963,15 +2991,20 @@ class NFToken_test : public beast::unit_test::suite token::owner(minter), token::destination(broker)); - // Cannot broker offers when the sell destination is not the buyer - // or the broker. - env(token::brokerOffers( - issuer, offerBuyerToBroker, offerMinterToBroker), - ter(tecNFTOKEN_BUY_SELL_MISMATCH)); - env.close(); - BEAST_EXPECT(ownerCount(env, issuer) == 0); - BEAST_EXPECT(ownerCount(env, minter) == 2); - BEAST_EXPECT(ownerCount(env, buyer) == 1); + { + // Cannot broker offers when the sell destination is not the + // buyer or the broker. + TER const expectTer = features[fixUnburnableNFToken] + ? tecNO_PERMISSION + : tecNFTOKEN_BUY_SELL_MISMATCH; + env(token::brokerOffers( + issuer, offerBuyerToBroker, offerMinterToBroker), + ter(expectTer)); + env.close(); + BEAST_EXPECT(ownerCount(env, issuer) == 0); + BEAST_EXPECT(ownerCount(env, minter) == 2); + BEAST_EXPECT(ownerCount(env, buyer) == 1); + } // Broker is successful if they are the destination of both offers. env(token::brokerOffers( @@ -6053,4 +6086,4 @@ class NFToken_test : public beast::unit_test::suite BEAST_DEFINE_TESTSUITE_PRIO(NFToken, tx, ripple, 2); -} // namespace ripple +} // namespace ripple \ No newline at end of file From ac78b7a9a7beccfe397c3725ca18a680d7502278 Mon Sep 17 00:00:00 2001 From: ledhed2222 Date: Mon, 13 Feb 2023 15:30:01 -0500 Subject: [PATCH 6/6] Rename to fixNonFungibleTokensV1_2 and some cosmetic changes (#4419) --- src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp | 36 ++++---- src/ripple/app/tx/impl/NFTokenBurn.cpp | 4 +- src/ripple/app/tx/impl/NFTokenCreateOffer.cpp | 2 +- src/ripple/protocol/Feature.h | 2 +- src/ripple/protocol/impl/Feature.cpp | 2 +- src/test/app/NFTokenBurn_test.cpp | 24 +++--- src/test/app/NFToken_test.cpp | 84 ++++++++++--------- 7 files changed, 74 insertions(+), 80 deletions(-) diff --git a/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp b/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp index c420bfc6197..61aa7e0629a 100644 --- a/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp +++ b/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp @@ -109,7 +109,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(fixUnburnableNFToken) && + if (ctx.view.rules().enabled(fixNonFungibleTokensV1_2) && ((*bo)[sfOwner] == (*so)[sfOwner])) return tecCANT_ACCEPT_OWN_NFTOKEN_OFFER; @@ -121,37 +121,29 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx) // If the buyer specified a destination if (auto const dest = bo->at(~sfDestination)) { - // fixUnburnableNFToken - if (ctx.view.rules().enabled(fixUnburnableNFToken)) + // 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)) { - // the destination may only be the account brokering the offer if (*dest != ctx.tx[sfAccount]) return tecNO_PERMISSION; } - else - { - // the destination must be the seller or the broker. - if (*dest != so->at(sfOwner) && *dest != ctx.tx[sfAccount]) - return tecNFTOKEN_BUY_SELL_MISMATCH; - } + else if (*dest != so->at(sfOwner) && *dest != ctx.tx[sfAccount]) + return tecNFTOKEN_BUY_SELL_MISMATCH; } // If the seller specified a destination if (auto const dest = so->at(~sfDestination)) { - // fixUnburnableNFToken - if (ctx.view.rules().enabled(fixUnburnableNFToken)) + // 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)) { - // the destination may only be the account brokering the offer if (*dest != ctx.tx[sfAccount]) return tecNO_PERMISSION; } - else - { - // the destination must be the buyer or the broker. - if (*dest != bo->at(sfOwner) && *dest != ctx.tx[sfAccount]) - return tecNFTOKEN_BUY_SELL_MISMATCH; - } + else if (*dest != bo->at(sfOwner) && *dest != ctx.tx[sfAccount]) + return tecNFTOKEN_BUY_SELL_MISMATCH; } // The broker can specify an amount that represents their cut; if they @@ -200,7 +192,7 @@ 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(fixUnburnableNFToken)) + if (ctx.view.rules().enabled(fixNonFungibleTokensV1_2)) { if (accountFunds( ctx.view, (*bo)[sfOwner], needed, fhZERO_IF_FROZEN, ctx.j) < @@ -243,7 +235,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(fixUnburnableNFToken)) + if (!ctx.view.rules().enabled(fixNonFungibleTokensV1_2)) { if (accountHolds( ctx.view, @@ -298,7 +290,7 @@ NFTokenAcceptOffer::pay( // 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(fixUnburnableNFToken)) + if (!view().rules().enabled(fixNonFungibleTokensV1_2)) return result; if (result != tesSUCCESS) return result; diff --git a/src/ripple/app/tx/impl/NFTokenBurn.cpp b/src/ripple/app/tx/impl/NFTokenBurn.cpp index e8693c7c6fb..99acfd61dca 100644 --- a/src/ripple/app/tx/impl/NFTokenBurn.cpp +++ b/src/ripple/app/tx/impl/NFTokenBurn.cpp @@ -77,7 +77,7 @@ NFTokenBurn::preclaim(PreclaimContext const& ctx) } } - if (!ctx.view.rules().enabled(fixUnburnableNFToken)) + 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. @@ -109,7 +109,7 @@ NFTokenBurn::doApply() view().update(issuer); } - if (ctx_.view().rules().enabled(fixUnburnableNFToken)) + 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 diff --git a/src/ripple/app/tx/impl/NFTokenCreateOffer.cpp b/src/ripple/app/tx/impl/NFTokenCreateOffer.cpp index ff8668e4488..6db31c69892 100644 --- a/src/ripple/app/tx/impl/NFTokenCreateOffer.cpp +++ b/src/ripple/app/tx/impl/NFTokenCreateOffer.cpp @@ -155,7 +155,7 @@ NFTokenCreateOffer::preclaim(PreclaimContext const& ctx) { // After this amendment, we allow an IOU issuer to make a buy offer // using their own currency. - if (ctx.view.rules().enabled(fixUnburnableNFToken)) + if (ctx.view.rules().enabled(fixNonFungibleTokensV1_2)) { if (accountFunds( ctx.view, diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 0bdfd224dda..d53d992d242 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -343,7 +343,7 @@ extern uint256 const featureImmediateOfferKilled; extern uint256 const featureDisallowIncoming; extern uint256 const featureXRPFees; extern uint256 const fixUniversalNumber; -extern uint256 const fixUnburnableNFToken; +extern uint256 const fixNonFungibleTokensV1_2; } // namespace ripple diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index f021ea4674d..4fb79e4cc48 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -453,7 +453,7 @@ REGISTER_FEATURE(ImmediateOfferKilled, Supported::yes, DefaultVote::no) REGISTER_FEATURE(DisallowIncoming, Supported::yes, DefaultVote::no); REGISTER_FEATURE(XRPFees, Supported::yes, DefaultVote::no); REGISTER_FIX (fixUniversalNumber, Supported::yes, DefaultVote::no); -REGISTER_FIX (fixUnburnableNFToken, Supported::yes, DefaultVote::no); +REGISTER_FIX (fixNonFungibleTokensV1_2, Supported::yes, DefaultVote::no); // The following amendments have been active for at least two years. Their // pre-amendment code has been removed and the identifiers are deprecated. diff --git a/src/test/app/NFTokenBurn_test.cpp b/src/test/app/NFTokenBurn_test.cpp index 4896932acd2..096fd5ce1e8 100644 --- a/src/test/app/NFTokenBurn_test.cpp +++ b/src/test/app/NFTokenBurn_test.cpp @@ -524,8 +524,8 @@ class NFTokenBurn_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 fixUnburnableNFToken goes live - if (!features[fixUnburnableNFToken]) + // more than 500 offers, before fixNonFungibleTokensV1_2 goes live + if (!features[fixNonFungibleTokensV1_2]) { Env env{*this, features}; @@ -620,10 +620,10 @@ class NFTokenBurn_test : public beast::unit_test::suite } // Test that up to 499 buy/sell offers will be removed when NFT is - // burned after fixUnburnableNFToken is enabled. This is to test that we - // can successfully remove all offers if the number of offers is less - // than 500. - if (features[fixUnburnableNFToken]) + // 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]) { Env env{*this, features}; @@ -673,8 +673,8 @@ class NFTokenBurn_test : public beast::unit_test::suite } // Test that up to 500 buy offers are removed when NFT is burned - // after fixUnburnableNFToken is enabled - if (features[fixUnburnableNFToken]) + // after fixNonFungibleTokensV1_2 is enabled + if (features[fixNonFungibleTokensV1_2]) { Env env{*this, features}; @@ -718,8 +718,8 @@ class NFTokenBurn_test : public beast::unit_test::suite } // Test that up to 500 buy/sell offers are removed when NFT is burned - // after fixUnburnableNFToken is enabled - if (features[fixUnburnableNFToken]) + // after fixNonFungibleTokensV1_2 is enabled + if (features[fixNonFungibleTokensV1_2]) { Env env{*this, features}; @@ -786,8 +786,8 @@ class NFTokenBurn_test : public beast::unit_test::suite FeatureBitset const all{supported_amendments()}; FeatureBitset const fixNFTDir{fixNFTokenDirV1}; - testWithFeats(all - fixUnburnableNFToken - fixNFTDir); - testWithFeats(all - fixUnburnableNFToken); + testWithFeats(all - fixNonFungibleTokensV1_2 - fixNFTDir); + testWithFeats(all - fixNonFungibleTokensV1_2); testWithFeats(all); } }; diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index d581a6d0d90..40202e07dce 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -2881,7 +2881,7 @@ class NFToken_test : public beast::unit_test::suite { // issuer cannot broker the offers, because they are not the // Destination. - TER const expectTer = features[fixUnburnableNFToken] + TER const expectTer = features[fixNonFungibleTokensV1_2] ? tecNO_PERMISSION : tecNFTOKEN_BUY_SELL_MISMATCH; env(token::brokerOffers( @@ -2931,7 +2931,7 @@ class NFToken_test : public beast::unit_test::suite { // Cannot broker offers when the sell destination is not the // buyer. - TER const expectTer = features[fixUnburnableNFToken] + TER const expectTer = features[fixNonFungibleTokensV1_2] ? tecNO_PERMISSION : tecNFTOKEN_BUY_SELL_MISMATCH; env(token::brokerOffers( @@ -2945,7 +2945,7 @@ class NFToken_test : public beast::unit_test::suite // amendment switch: When enabled the broker fails, when // disabled the broker succeeds if the destination is the buyer. - TER const eexpectTer = features[fixUnburnableNFToken] + TER const eexpectTer = features[fixNonFungibleTokensV1_2] ? tecNO_PERMISSION : TER(tesSUCCESS); env(token::brokerOffers( @@ -2953,7 +2953,7 @@ class NFToken_test : public beast::unit_test::suite ter(eexpectTer)); env.close(); - if (features[fixUnburnableNFToken]) + if (features[fixNonFungibleTokensV1_2]) // Buyer is successful with acceptOffer. env(token::acceptBuyOffer(buyer, offerMinterToBuyer)); env.close(); @@ -2994,7 +2994,7 @@ class NFToken_test : public beast::unit_test::suite { // Cannot broker offers when the sell destination is not the // buyer or the broker. - TER const expectTer = features[fixUnburnableNFToken] + TER const expectTer = features[fixNonFungibleTokensV1_2] ? tecNO_PERMISSION : tecNFTOKEN_BUY_SELL_MISMATCH; env(token::brokerOffers( @@ -3861,7 +3861,8 @@ class NFToken_test : public beast::unit_test::suite using namespace test::jtx; for (auto const& tweakedFeatures : - {features - fixUnburnableNFToken, features | fixUnburnableNFToken}) + {features - fixNonFungibleTokensV1_2, + features | fixNonFungibleTokensV1_2}) { Env env{*this, tweakedFeatures}; @@ -4400,7 +4401,7 @@ class NFToken_test : public beast::unit_test::suite token::owner(minter)); env.close(); - if (tweakedFeatures[fixUnburnableNFToken]) + if (tweakedFeatures[fixNonFungibleTokensV1_2]) { env(token::brokerOffers( broker, buyOfferIndex, minterOfferIndex), @@ -4946,12 +4947,12 @@ class NFToken_test : public beast::unit_test::suite IOU const gwXAU(gw["XAU"]); // Test both with and without fixNFTokenNegOffer and - // fixUnburnableNFToken. Need to turn off fixUnburnableNFToken as well - // because that amendment came later and addressed the acceptance - // side of this issue. + // 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 - - fixUnburnableNFToken, + fixNonFungibleTokensV1_2, features - fixNFTokenNegOffer - featureNonFungibleTokensV1_1, features | fixNFTokenNegOffer}) { @@ -5042,7 +5043,7 @@ class NFToken_test : public beast::unit_test::suite } { // 1. If fixNFTokenNegOffer is enabled get tecOBJECT_NOT_FOUND - // 2. If it is not enabled, but fixUnburnableNFToken is + // 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] @@ -5184,7 +5185,8 @@ class NFToken_test : public beast::unit_test::suite testcase("Payments with IOU transfer fees"); for (auto const& tweakedFeatures : - {features - fixUnburnableNFToken, features | fixUnburnableNFToken}) + {features - fixNonFungibleTokensV1_2, + features | fixNonFungibleTokensV1_2}) { Env env{*this, tweakedFeatures}; @@ -5336,13 +5338,13 @@ class NFToken_test : public beast::unit_test::suite auto const nftID = mintNFT(minter); auto const offerID = createSellOffer(minter, nftID, gwXAU(1000)); - auto const sellTER = tweakedFeatures[fixUnburnableNFToken] + auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] ? static_cast(tecINSUFFICIENT_FUNDS) : static_cast(tesSUCCESS); env(token::acceptSellOffer(buyer, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixUnburnableNFToken]) + if (tweakedFeatures[fixNonFungibleTokensV1_2]) expectInitialState(); else { @@ -5360,13 +5362,13 @@ class NFToken_test : public beast::unit_test::suite auto const nftID = mintNFT(minter); auto const offerID = createBuyOffer(buyer, minter, nftID, gwXAU(1000)); - auto const sellTER = tweakedFeatures[fixUnburnableNFToken] + auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] ? static_cast(tecINSUFFICIENT_FUNDS) : static_cast(tesSUCCESS); env(token::acceptBuyOffer(minter, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixUnburnableNFToken]) + if (tweakedFeatures[fixNonFungibleTokensV1_2]) expectInitialState(); else { @@ -5384,13 +5386,13 @@ class NFToken_test : public beast::unit_test::suite reinitializeTrustLineBalances(); auto const nftID = mintNFT(minter); auto const offerID = createSellOffer(minter, nftID, gwXAU(995)); - auto const sellTER = tweakedFeatures[fixUnburnableNFToken] + auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] ? static_cast(tecINSUFFICIENT_FUNDS) : static_cast(tesSUCCESS); env(token::acceptSellOffer(buyer, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixUnburnableNFToken]) + if (tweakedFeatures[fixNonFungibleTokensV1_2]) expectInitialState(); else { @@ -5408,13 +5410,13 @@ class NFToken_test : public beast::unit_test::suite auto const nftID = mintNFT(minter); auto const offerID = createBuyOffer(buyer, minter, nftID, gwXAU(995)); - auto const sellTER = tweakedFeatures[fixUnburnableNFToken] + auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] ? static_cast(tecINSUFFICIENT_FUNDS) : static_cast(tesSUCCESS); env(token::acceptBuyOffer(minter, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixUnburnableNFToken]) + if (tweakedFeatures[fixNonFungibleTokensV1_2]) expectInitialState(); else { @@ -5509,13 +5511,13 @@ class NFToken_test : public beast::unit_test::suite auto const nftID = mintNFT(minter); auto const offerID = createSellOffer(minter, nftID, gwXAU(1000)); - auto const sellTER = tweakedFeatures[fixUnburnableNFToken] + auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] ? static_cast(tesSUCCESS) : static_cast(tecINSUFFICIENT_FUNDS); env(token::acceptSellOffer(gw, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixUnburnableNFToken]) + if (tweakedFeatures[fixNonFungibleTokensV1_2]) { BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(1000)); BEAST_EXPECT( @@ -5530,18 +5532,18 @@ class NFToken_test : public beast::unit_test::suite reinitializeTrustLineBalances(); auto const nftID = mintNFT(minter); - auto const offerTER = tweakedFeatures[fixUnburnableNFToken] + auto const offerTER = tweakedFeatures[fixNonFungibleTokensV1_2] ? static_cast(tesSUCCESS) : static_cast(tecUNFUNDED_OFFER); auto const offerID = createBuyOffer(gw, minter, nftID, gwXAU(1000), {offerTER}); - auto const sellTER = tweakedFeatures[fixUnburnableNFToken] + auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] ? static_cast(tesSUCCESS) : static_cast(tecOBJECT_NOT_FOUND); env(token::acceptBuyOffer(minter, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixUnburnableNFToken]) + if (tweakedFeatures[fixNonFungibleTokensV1_2]) { BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(1000)); BEAST_EXPECT( @@ -5557,13 +5559,13 @@ class NFToken_test : public beast::unit_test::suite auto const nftID = mintNFT(minter); auto const offerID = createSellOffer(minter, nftID, gwXAU(5000)); - auto const sellTER = tweakedFeatures[fixUnburnableNFToken] + auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] ? static_cast(tesSUCCESS) : static_cast(tecINSUFFICIENT_FUNDS); env(token::acceptSellOffer(gw, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixUnburnableNFToken]) + if (tweakedFeatures[fixNonFungibleTokensV1_2]) { BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(5000)); BEAST_EXPECT( @@ -5578,18 +5580,18 @@ class NFToken_test : public beast::unit_test::suite reinitializeTrustLineBalances(); auto const nftID = mintNFT(minter); - auto const offerTER = tweakedFeatures[fixUnburnableNFToken] + auto const offerTER = tweakedFeatures[fixNonFungibleTokensV1_2] ? static_cast(tesSUCCESS) : static_cast(tecUNFUNDED_OFFER); auto const offerID = createBuyOffer(gw, minter, nftID, gwXAU(5000), {offerTER}); - auto const sellTER = tweakedFeatures[fixUnburnableNFToken] + auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] ? static_cast(tesSUCCESS) : static_cast(tecOBJECT_NOT_FOUND); env(token::acceptBuyOffer(minter, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixUnburnableNFToken]) + if (tweakedFeatures[fixNonFungibleTokensV1_2]) { BEAST_EXPECT(env.balance(minter, gwXAU) == gwXAU(5000)); BEAST_EXPECT( @@ -5731,13 +5733,13 @@ class NFToken_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[fixUnburnableNFToken] + auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] ? static_cast(tecINSUFFICIENT_FUNDS) : static_cast(tesSUCCESS); env(token::acceptSellOffer(buyer, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixUnburnableNFToken]) + if (tweakedFeatures[fixNonFungibleTokensV1_2]) expectInitialState(); else { @@ -5767,14 +5769,14 @@ class NFToken_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[fixUnburnableNFToken] + auto const sellTER = tweakedFeatures[fixNonFungibleTokensV1_2] ? static_cast(tecINSUFFICIENT_FUNDS) : static_cast(tesSUCCESS); env(token::acceptBuyOffer(secondarySeller, offerID), ter(sellTER)); env.close(); - if (tweakedFeatures[fixUnburnableNFToken]) + if (tweakedFeatures[fixNonFungibleTokensV1_2]) expectInitialState(); else { @@ -5940,7 +5942,7 @@ class NFToken_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 fixUnburnableNFToken amendment + // This unit test verifies that the fixNonFungibleTokensV1_2 amendment // fixes that bug. testcase("Brokered sale to self"); @@ -6006,7 +6008,7 @@ class NFToken_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[fixUnburnableNFToken] + TER expectTer = features[fixNonFungibleTokensV1_2] ? TER(tecCANT_ACCEPT_OWN_NFTOKEN_OFFER) : TER(tesSUCCESS); env(token::brokerOffers(broker, bobBuyOfferIndex, bobSellOfferIndex), @@ -6077,13 +6079,13 @@ class NFToken_test : public beast::unit_test::suite FeatureBitset const all{supported_amendments()}; FeatureBitset const fixNFTDir{fixNFTokenDirV1}; - testWithFeats(all - fixNFTDir - fixUnburnableNFToken); - testWithFeats(all - disallowIncoming - fixUnburnableNFToken); - testWithFeats(all - fixUnburnableNFToken); + testWithFeats(all - fixNFTDir - fixNonFungibleTokensV1_2); + testWithFeats(all - disallowIncoming - fixNonFungibleTokensV1_2); + testWithFeats(all - fixNonFungibleTokensV1_2); testWithFeats(all); } }; BEAST_DEFINE_TESTSUITE_PRIO(NFToken, tx, ripple, 2); -} // namespace ripple \ No newline at end of file +} // namespace ripple