From df092fc0b6d7c3eeefe2e174f893c0f6215db100 Mon Sep 17 00:00:00 2001 From: tequ Date: Sun, 19 Nov 2023 17:40:17 +0900 Subject: [PATCH 01/13] NFTokenMIntOffer initial --- src/ripple/app/tx/impl/NFTokenMint.cpp | 139 +++++++++++++++++++++++-- src/ripple/protocol/Feature.h | 3 +- src/ripple/protocol/impl/Feature.cpp | 3 +- src/ripple/protocol/impl/TxFormats.cpp | 3 + src/test/app/NFToken_test.cpp | 72 ++++++++++++- src/test/jtx/impl/token.cpp | 6 ++ src/test/jtx/token.h | 15 +++ 7 files changed, 228 insertions(+), 13 deletions(-) diff --git a/src/ripple/app/tx/impl/NFTokenMint.cpp b/src/ripple/app/tx/impl/NFTokenMint.cpp index c26fb1fb12a..73994706f4d 100644 --- a/src/ripple/app/tx/impl/NFTokenMint.cpp +++ b/src/ripple/app/tx/impl/NFTokenMint.cpp @@ -37,6 +37,9 @@ NFTokenMint::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureNonFungibleTokensV1)) return temDISABLED; + if (!ctx.rules.enabled(featureNFTokenMintOffer) && (ctx.tx[sfAmount] || ctx.tx[~sfDestination] || ctx.tx[~sfExpiration])) + return temDISABLED; + if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) return ret; @@ -80,6 +83,29 @@ NFTokenMint::preflight(PreflightContext const& ctx) return temMALFORMED; } + // Amount field should be set if Destination and/or Expiration is set + if (ctx.tx[~sfDestination] || ctx.tx[~sfExpiration]) + { + if (!ctx.tx.isFieldPresent(sfAmount)) + return temMALFORMED; + } + + if(STAmount const amount = ctx.tx[sfAmount]) + { + if (amount.negative() && ctx.rules.enabled(fixNFTokenNegOffer)) + // An offer for a negative amount makes no sense. + return temBAD_AMOUNT; + + if (!isXRP(amount)) + { + if (ctx.tx.isFlag(tfOnlyXRP)) + return temBAD_AMOUNT; + + if (!amount) + return temBAD_AMOUNT; + } + } + return preflight2(ctx); } @@ -146,6 +172,52 @@ NFTokenMint::preclaim(PreclaimContext const& ctx) return tecNO_PERMISSION; } + if(STAmount const amount = ctx.tx[sfAmount]){ + + if (hasExpired(ctx.view, ctx.tx[~sfExpiration])) + return tecEXPIRED; + + if (!(ctx.tx.getFlags() & nft::flagCreateTrustLines) && !amount.native() && + ctx.tx[~sfTransferFee]) + { + auto const nftIssuer = ctx.tx[~sfIssuer].value_or(ctx.tx[sfAccount]); + if (!ctx.view.exists(keylet::account(nftIssuer))) + return tecNO_ISSUER; + + if (!ctx.view.exists(keylet::line(nftIssuer, amount.issue()))) + return tecNO_LINE; + + if (isFrozen( + ctx.view, nftIssuer, amount.getCurrency(), amount.getIssuer())) + return tecFROZEN; + } + + if (isFrozen( + ctx.view, + ctx.tx[sfAccount], + amount.getCurrency(), + amount.getIssuer())) + return tecFROZEN; + + if (auto const destination = ctx.tx[~sfDestination]) + { + // If a destination is specified, the destination must already be in + // the ledger. + auto const sleDst = ctx.view.read(keylet::account(*destination)); + + if (!sleDst) + return tecNO_DST; + + // check if the destination has disallowed incoming offers + if (ctx.view.rules().enabled(featureDisallowIncoming)) + { + // flag cannot be set unless amendment is enabled but + // out of an abundance of caution check anyway + if (sleDst->getFlags() & lsfDisallowIncomingNFTokenOffer) + return tecNO_PERMISSION; + } + } + } return tesSUCCESS; } @@ -237,19 +309,21 @@ NFTokenMint::doApply() if (nfTokenTemplate == nullptr) // Should never happen. return tecINTERNAL; + + auto const nftokenID = createNFTokenID( + static_cast(ctx_.tx.getFlags() & 0x0000FFFF), + ctx_.tx[~sfTransferFee].value_or(0), + issuer, + nft::toTaxon(ctx_.tx[sfNFTokenTaxon]), + tokenSeq.value()); STObject newToken( *nfTokenTemplate, sfNFToken, - [this, &issuer, &tokenSeq](STObject& object) { + [this, &nftokenID](STObject& object) { object.setFieldH256( sfNFTokenID, - createNFTokenID( - static_cast(ctx_.tx.getFlags() & 0x0000FFFF), - ctx_.tx[~sfTransferFee].value_or(0), - issuer, - nft::toTaxon(ctx_.tx[sfNFTokenTaxon]), - tokenSeq.value())); + nftokenID); if (auto const uri = ctx_.tx[~sfURI]) object.setFieldVL(sfURI, *uri); @@ -259,6 +333,57 @@ NFTokenMint::doApply() nft::insertToken(ctx_.view(), account_, std::move(newToken)); ret != tesSUCCESS) return ret; + + if (ctx_.tx.isFieldPresent(sfAmount)) { + auto const offerID = + keylet::nftoffer(account_, ctx_.tx.getSeqProxy().value()); + + // Create the offer: + { + // Token offers are always added to the owner's owner directory: + auto const ownerNode = view().dirInsert( + keylet::ownerDir(account_), offerID, describeOwnerDir(account_)); + + if (!ownerNode) + return tecDIR_FULL; + + bool const isSellOffer = true; + + // Token offers are also added to the token's buy or sell offer + // directory + auto const offerNode = view().dirInsert( + keylet::nft_sells(nftokenID), + offerID, + [&nftokenID](std::shared_ptr const& sle) { + (*sle)[sfFlags] =lsfNFTokenSellOffers; + (*sle)[sfNFTokenID] = nftokenID; + }); + + if (!offerNode) + return tecDIR_FULL; + + std::uint32_t sleFlags = 0; + + if (isSellOffer) + sleFlags |= lsfSellNFToken; + + auto offer = std::make_shared(offerID); + (*offer)[sfOwner] = account_; + (*offer)[sfNFTokenID] = nftokenID; + (*offer)[sfAmount] = ctx_.tx[sfAmount]; + (*offer)[sfFlags] = sleFlags; + (*offer)[sfOwnerNode] = *ownerNode; + (*offer)[sfNFTokenOfferNode] = *offerNode; + + if (auto const expiration = ctx_.tx[~sfExpiration]) + (*offer)[sfExpiration] = *expiration; + + if (auto const destination = ctx_.tx[~sfDestination]) + (*offer)[sfDestination] = *destination; + + view().insert(offer); + } + } // Only check the reserve if the owner count actually changed. This // allows NFTs to be added to the page (and burn fees) without diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 3bdfcb15c59..6b87f8df931 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 = 65; +static constexpr std::size_t numFeatures = 66; /** 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 featureAMM; +extern uint256 const featureNFTokenMintOffer; extern uint256 const fixUniversalNumber; extern uint256 const fixNonFungibleTokensV1_2; extern uint256 const fixNFTokenRemint; diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 25033d4336e..88e8fef0324 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -458,7 +458,8 @@ REGISTER_FEATURE(AMM, Supported::yes, VoteBehavior::De REGISTER_FEATURE(XChainBridge, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FIX (fixDisallowIncomingV1, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FEATURE(DID, Supported::yes, VoteBehavior::DefaultNo); -REGISTER_FIX(fixFillOrKill, Supported::yes, VoteBehavior::DefaultNo); +REGISTER_FIX (fixFillOrKill, Supported::yes, VoteBehavior::DefaultNo); +REGISTER_FEATURE(NFTokenMintOffer, Supported::yes, VoteBehavior::DefaultNo); // The following amendments are obsolete, but must remain supported // because they could potentially get enabled. diff --git a/src/ripple/protocol/impl/TxFormats.cpp b/src/ripple/protocol/impl/TxFormats.cpp index 7be8ca741e2..be2d9de3c0f 100644 --- a/src/ripple/protocol/impl/TxFormats.cpp +++ b/src/ripple/protocol/impl/TxFormats.cpp @@ -333,6 +333,9 @@ TxFormats::TxFormats() {sfTransferFee, soeOPTIONAL}, {sfIssuer, soeOPTIONAL}, {sfURI, soeOPTIONAL}, + {sfAmount, soeOPTIONAL}, + {sfDestination, soeOPTIONAL}, + {sfExpiration, soeOPTIONAL}, }, commonFields); diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 2f029c5db1f..4c4f25d0a0c 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -6566,6 +6566,68 @@ class NFToken0_test : public beast::unit_test::suite } } + void + testFeatNFTokenMintAndOffer(FeatureBitset features) + { + + testcase("NFTokenMint and Create NFTokenOffer"); + + using namespace test::jtx; + + if (!features[featureNFTokenMintOffer]) + { + Env env{*this, features}; + Account const alice("alice"); + Account const becky("becky"); + + env.fund(XRP(10000), alice, becky); + env.close(); + + env(token::mint(alice), token::amount(XRP(10000)), ter(temDISABLED)); + env.close(); + } + + if(features[featureNFTokenMintOffer]) + { + Env env{*this, features}; + Account const alice("alice"); + Account const becky("becky"); + + env.fund(XRP(10000), alice, becky); + env.close(); + + // Destination field specified but Amount field not specified + env(token::mint(alice), token::destination(becky), ter(temMALFORMED)); + env.close(); + + // Expiration field specified but Amount field not specified + env(token::mint(alice), token::expiration(lastClose(env) + 25), ter(temMALFORMED)); + env.close(); + + // Amount field specified + BEAST_EXPECT(ownerCount(env, alice) == 1); + env(token::mint(alice), token::amount(XRP(10))); + BEAST_EXPECT(ownerCount(env, alice) == 2); + env.close(); + + // Amount field and Destination field, Expiration field specified + env(token::mint(alice), token::amount(XRP(10)), token::destination(becky), token::expiration(lastClose(env) + 25)); + env.close(); + + env(token::mint(alice), token::amount(XRP(10)), token::destination(becky), token::expiration(lastClose(env) + 25)); + uint256 const offerAliceSellsToBecky = + keylet::nftoffer(alice, env.seq(alice)).key; + env(token::cancelOffer(alice, {offerAliceSellsToBecky})); + env.close(); + + env(token::mint(becky), token::amount(XRP(10)), token::destination(alice), token::expiration(lastClose(env) + 25)); + uint256 const offerBeckySellsToAlice = + keylet::nftoffer(becky, env.seq(becky)).key; + env(token::cancelOffer(alice, {offerBeckySellsToAlice})); + env.close(); + } + } + void testTxJsonMetaFields(FeatureBitset features) { @@ -6831,6 +6893,7 @@ class NFToken0_test : public beast::unit_test::suite testBrokeredSaleToSelf(features); testFixNFTokenRemint(features); testTxJsonMetaFields(features); + testFeatNFTokenMintAndOffer(features); } public: @@ -6842,11 +6905,12 @@ class NFToken0_test : public beast::unit_test::suite static FeatureBitset const fixNFTDir{fixNFTokenDirV1}; static std::array const feats{ - all - fixNFTDir - fixNonFungibleTokensV1_2 - fixNFTokenRemint, + all - fixNFTDir - fixNonFungibleTokensV1_2 - fixNFTokenRemint - featureNFTokenMintOffer, all - disallowIncoming - fixNonFungibleTokensV1_2 - - fixNFTokenRemint, - all - fixNonFungibleTokensV1_2 - fixNFTokenRemint, - all - fixNFTokenRemint, + fixNFTokenRemint - featureNFTokenMintOffer, + all - fixNonFungibleTokensV1_2 - fixNFTokenRemint - featureNFTokenMintOffer, + all - fixNFTokenRemint - featureNFTokenMintOffer, + all - featureNFTokenMintOffer, all}; if (BEAST_EXPECT(instance < feats.size())) diff --git a/src/test/jtx/impl/token.cpp b/src/test/jtx/impl/token.cpp index 6c5cae4147a..12b738ca42f 100644 --- a/src/test/jtx/impl/token.cpp +++ b/src/test/jtx/impl/token.cpp @@ -57,6 +57,12 @@ uri::operator()(Env& env, JTx& jt) const jt.jv[sfURI.jsonName] = uri_; } +void +amount::operator()(Env& env, JTx& jt) const +{ + jt.jv[sfAmount.jsonName] = amount_.getJson(JsonOptions::none); +} + uint256 getNextID( jtx::Env const& env, diff --git a/src/test/jtx/token.h b/src/test/jtx/token.h index 150ddfab0ea..957e499cefa 100644 --- a/src/test/jtx/token.h +++ b/src/test/jtx/token.h @@ -83,6 +83,21 @@ class uri operator()(Env&, JTx& jtx) const; }; +/** Sets the optional amount field on an NFTokenMint. */ +class amount +{ +private: + STAmount const amount_; + +public: + explicit amount(STAmount const amount) : amount_(amount) + { + } + + void + operator()(Env&, JTx& jtx) const; +}; + /** Get the next NFTokenID that will be issued. */ uint256 getNextID( From 351c1353be3b8051856a838f6dae5e11df9f971c Mon Sep 17 00:00:00 2001 From: tequ Date: Sun, 19 Nov 2023 17:40:17 +0900 Subject: [PATCH 02/13] clang-format --- src/ripple/app/tx/impl/NFTokenMint.cpp | 212 +++++++++++++------------ src/test/app/NFToken_test.cpp | 49 +++--- 2 files changed, 139 insertions(+), 122 deletions(-) diff --git a/src/ripple/app/tx/impl/NFTokenMint.cpp b/src/ripple/app/tx/impl/NFTokenMint.cpp index 73994706f4d..218df865482 100644 --- a/src/ripple/app/tx/impl/NFTokenMint.cpp +++ b/src/ripple/app/tx/impl/NFTokenMint.cpp @@ -37,7 +37,8 @@ NFTokenMint::preflight(PreflightContext const& ctx) if (!ctx.rules.enabled(featureNonFungibleTokensV1)) return temDISABLED; - if (!ctx.rules.enabled(featureNFTokenMintOffer) && (ctx.tx[sfAmount] || ctx.tx[~sfDestination] || ctx.tx[~sfExpiration])) + if (!ctx.rules.enabled(featureNFTokenMintOffer) && + (ctx.tx[sfAmount] || ctx.tx[~sfDestination] || ctx.tx[~sfExpiration])) return temDISABLED; if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) @@ -90,7 +91,7 @@ NFTokenMint::preflight(PreflightContext const& ctx) return temMALFORMED; } - if(STAmount const amount = ctx.tx[sfAmount]) + if (STAmount const amount = ctx.tx[sfAmount]) { if (amount.negative() && ctx.rules.enabled(fixNFTokenNegOffer)) // An offer for a negative amount makes no sense. @@ -172,51 +173,55 @@ NFTokenMint::preclaim(PreclaimContext const& ctx) return tecNO_PERMISSION; } - if(STAmount const amount = ctx.tx[sfAmount]){ - - if (hasExpired(ctx.view, ctx.tx[~sfExpiration])) - return tecEXPIRED; - - if (!(ctx.tx.getFlags() & nft::flagCreateTrustLines) && !amount.native() && - ctx.tx[~sfTransferFee]) - { - auto const nftIssuer = ctx.tx[~sfIssuer].value_or(ctx.tx[sfAccount]); - if (!ctx.view.exists(keylet::account(nftIssuer))) - return tecNO_ISSUER; - - if (!ctx.view.exists(keylet::line(nftIssuer, amount.issue()))) - return tecNO_LINE; - - if (isFrozen( - ctx.view, nftIssuer, amount.getCurrency(), amount.getIssuer())) - return tecFROZEN; - } - - if (isFrozen( - ctx.view, - ctx.tx[sfAccount], - amount.getCurrency(), - amount.getIssuer())) - return tecFROZEN; - - if (auto const destination = ctx.tx[~sfDestination]) - { - // If a destination is specified, the destination must already be in - // the ledger. - auto const sleDst = ctx.view.read(keylet::account(*destination)); - - if (!sleDst) - return tecNO_DST; - - // check if the destination has disallowed incoming offers - if (ctx.view.rules().enabled(featureDisallowIncoming)) - { - // flag cannot be set unless amendment is enabled but - // out of an abundance of caution check anyway - if (sleDst->getFlags() & lsfDisallowIncomingNFTokenOffer) - return tecNO_PERMISSION; - } - } + if (STAmount const amount = ctx.tx[sfAmount]) + { + if (hasExpired(ctx.view, ctx.tx[~sfExpiration])) + return tecEXPIRED; + + if (!(ctx.tx.getFlags() & nft::flagCreateTrustLines) && + !amount.native() && ctx.tx[~sfTransferFee]) + { + auto const nftIssuer = + ctx.tx[~sfIssuer].value_or(ctx.tx[sfAccount]); + if (!ctx.view.exists(keylet::account(nftIssuer))) + return tecNO_ISSUER; + + if (!ctx.view.exists(keylet::line(nftIssuer, amount.issue()))) + return tecNO_LINE; + + if (isFrozen( + ctx.view, + nftIssuer, + amount.getCurrency(), + amount.getIssuer())) + return tecFROZEN; + } + + if (isFrozen( + ctx.view, + ctx.tx[sfAccount], + amount.getCurrency(), + amount.getIssuer())) + return tecFROZEN; + + if (auto const destination = ctx.tx[~sfDestination]) + { + // If a destination is specified, the destination must already be in + // the ledger. + auto const sleDst = ctx.view.read(keylet::account(*destination)); + + if (!sleDst) + return tecNO_DST; + + // check if the destination has disallowed incoming offers + if (ctx.view.rules().enabled(featureDisallowIncoming)) + { + // flag cannot be set unless amendment is enabled but + // out of an abundance of caution check anyway + if (sleDst->getFlags() & lsfDisallowIncomingNFTokenOffer) + return tecNO_PERMISSION; + } + } } return tesSUCCESS; } @@ -309,7 +314,7 @@ NFTokenMint::doApply() if (nfTokenTemplate == nullptr) // Should never happen. return tecINTERNAL; - + auto const nftokenID = createNFTokenID( static_cast(ctx_.tx.getFlags() & 0x0000FFFF), ctx_.tx[~sfTransferFee].value_or(0), @@ -318,12 +323,8 @@ NFTokenMint::doApply() tokenSeq.value()); STObject newToken( - *nfTokenTemplate, - sfNFToken, - [this, &nftokenID](STObject& object) { - object.setFieldH256( - sfNFTokenID, - nftokenID); + *nfTokenTemplate, sfNFToken, [this, &nftokenID](STObject& object) { + object.setFieldH256(sfNFTokenID, nftokenID); if (auto const uri = ctx_.tx[~sfURI]) object.setFieldVL(sfURI, *uri); @@ -333,56 +334,59 @@ NFTokenMint::doApply() nft::insertToken(ctx_.view(), account_, std::move(newToken)); ret != tesSUCCESS) return ret; - - if (ctx_.tx.isFieldPresent(sfAmount)) { - auto const offerID = - keylet::nftoffer(account_, ctx_.tx.getSeqProxy().value()); - - // Create the offer: - { - // Token offers are always added to the owner's owner directory: - auto const ownerNode = view().dirInsert( - keylet::ownerDir(account_), offerID, describeOwnerDir(account_)); - - if (!ownerNode) - return tecDIR_FULL; - - bool const isSellOffer = true; - - // Token offers are also added to the token's buy or sell offer - // directory - auto const offerNode = view().dirInsert( - keylet::nft_sells(nftokenID), - offerID, - [&nftokenID](std::shared_ptr const& sle) { - (*sle)[sfFlags] =lsfNFTokenSellOffers; - (*sle)[sfNFTokenID] = nftokenID; - }); - - if (!offerNode) - return tecDIR_FULL; - - std::uint32_t sleFlags = 0; - - if (isSellOffer) - sleFlags |= lsfSellNFToken; - - auto offer = std::make_shared(offerID); - (*offer)[sfOwner] = account_; - (*offer)[sfNFTokenID] = nftokenID; - (*offer)[sfAmount] = ctx_.tx[sfAmount]; - (*offer)[sfFlags] = sleFlags; - (*offer)[sfOwnerNode] = *ownerNode; - (*offer)[sfNFTokenOfferNode] = *offerNode; - - if (auto const expiration = ctx_.tx[~sfExpiration]) - (*offer)[sfExpiration] = *expiration; - - if (auto const destination = ctx_.tx[~sfDestination]) - (*offer)[sfDestination] = *destination; - - view().insert(offer); - } + + if (ctx_.tx.isFieldPresent(sfAmount)) + { + auto const offerID = + keylet::nftoffer(account_, ctx_.tx.getSeqProxy().value()); + + // Create the offer: + { + // Token offers are always added to the owner's owner directory: + auto const ownerNode = view().dirInsert( + keylet::ownerDir(account_), + offerID, + describeOwnerDir(account_)); + + if (!ownerNode) + return tecDIR_FULL; + + bool const isSellOffer = true; + + // Token offers are also added to the token's buy or sell offer + // directory + auto const offerNode = view().dirInsert( + keylet::nft_sells(nftokenID), + offerID, + [&nftokenID](std::shared_ptr const& sle) { + (*sle)[sfFlags] = lsfNFTokenSellOffers; + (*sle)[sfNFTokenID] = nftokenID; + }); + + if (!offerNode) + return tecDIR_FULL; + + std::uint32_t sleFlags = 0; + + if (isSellOffer) + sleFlags |= lsfSellNFToken; + + auto offer = std::make_shared(offerID); + (*offer)[sfOwner] = account_; + (*offer)[sfNFTokenID] = nftokenID; + (*offer)[sfAmount] = ctx_.tx[sfAmount]; + (*offer)[sfFlags] = sleFlags; + (*offer)[sfOwnerNode] = *ownerNode; + (*offer)[sfNFTokenOfferNode] = *offerNode; + + if (auto const expiration = ctx_.tx[~sfExpiration]) + (*offer)[sfExpiration] = *expiration; + + if (auto const destination = ctx_.tx[~sfDestination]) + (*offer)[sfDestination] = *destination; + + view().insert(offer); + } } // Only check the reserve if the owner count actually changed. This diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 4c4f25d0a0c..3ae3097bb37 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -6569,58 +6569,71 @@ class NFToken0_test : public beast::unit_test::suite void testFeatNFTokenMintAndOffer(FeatureBitset features) { - testcase("NFTokenMint and Create NFTokenOffer"); using namespace test::jtx; - - if (!features[featureNFTokenMintOffer]) + + if (!features[featureNFTokenMintOffer]) { Env env{*this, features}; Account const alice("alice"); Account const becky("becky"); - + env.fund(XRP(10000), alice, becky); env.close(); - - env(token::mint(alice), token::amount(XRP(10000)), ter(temDISABLED)); - env.close(); + + env(token::mint(alice), + token::amount(XRP(10000)), + ter(temDISABLED)); } - if(features[featureNFTokenMintOffer]) + if (features[featureNFTokenMintOffer]) { Env env{*this, features}; Account const alice("alice"); Account const becky("becky"); - + env.fund(XRP(10000), alice, becky); env.close(); - + // Destination field specified but Amount field not specified - env(token::mint(alice), token::destination(becky), ter(temMALFORMED)); + env(token::mint(alice), + token::destination(becky), + ter(temMALFORMED)); env.close(); // Expiration field specified but Amount field not specified - env(token::mint(alice), token::expiration(lastClose(env) + 25), ter(temMALFORMED)); + env(token::mint(alice), + token::expiration(lastClose(env) + 25), + ter(temMALFORMED)); env.close(); - + // Amount field specified BEAST_EXPECT(ownerCount(env, alice) == 1); env(token::mint(alice), token::amount(XRP(10))); BEAST_EXPECT(ownerCount(env, alice) == 2); env.close(); - + // Amount field and Destination field, Expiration field specified - env(token::mint(alice), token::amount(XRP(10)), token::destination(becky), token::expiration(lastClose(env) + 25)); + env(token::mint(alice), + token::amount(XRP(10)), + token::destination(becky), + token::expiration(lastClose(env) + 25)); env.close(); - - env(token::mint(alice), token::amount(XRP(10)), token::destination(becky), token::expiration(lastClose(env) + 25)); + + env(token::mint(alice), + token::amount(XRP(10)), + token::destination(becky), + token::expiration(lastClose(env) + 25)); uint256 const offerAliceSellsToBecky = keylet::nftoffer(alice, env.seq(alice)).key; env(token::cancelOffer(alice, {offerAliceSellsToBecky})); env.close(); - env(token::mint(becky), token::amount(XRP(10)), token::destination(alice), token::expiration(lastClose(env) + 25)); + env(token::mint(becky), + token::amount(XRP(10)), + token::destination(alice), + token::expiration(lastClose(env) + 25)); uint256 const offerBeckySellsToAlice = keylet::nftoffer(becky, env.seq(becky)).key; env(token::cancelOffer(alice, {offerBeckySellsToAlice})); From 650af4873d520581f3f202b09e348fa6b19d4b70 Mon Sep 17 00:00:00 2001 From: tequ Date: Fri, 1 Dec 2023 16:50:44 +0900 Subject: [PATCH 03/13] clang-format --- src/test/app/NFToken_test.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 3ae3097bb37..8ce463a2d31 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -6918,10 +6918,12 @@ class NFToken0_test : public beast::unit_test::suite static FeatureBitset const fixNFTDir{fixNFTokenDirV1}; static std::array const feats{ - all - fixNFTDir - fixNonFungibleTokensV1_2 - fixNFTokenRemint - featureNFTokenMintOffer, + all - fixNFTDir - fixNonFungibleTokensV1_2 - fixNFTokenRemint - + featureNFTokenMintOffer, all - disallowIncoming - fixNonFungibleTokensV1_2 - fixNFTokenRemint - featureNFTokenMintOffer, - all - fixNonFungibleTokensV1_2 - fixNFTokenRemint - featureNFTokenMintOffer, + all - fixNonFungibleTokensV1_2 - fixNFTokenRemint - + featureNFTokenMintOffer, all - fixNFTokenRemint - featureNFTokenMintOffer, all - featureNFTokenMintOffer, all}; From 5815a570561eb5390372943383ba9e1f9937b6c0 Mon Sep 17 00:00:00 2001 From: tequ Date: Tue, 5 Dec 2023 23:35:21 +0900 Subject: [PATCH 04/13] Update Feature.h --- src/ripple/protocol/Feature.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 6b87f8df931..b635af67031 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -343,7 +343,6 @@ extern uint256 const featureImmediateOfferKilled; extern uint256 const featureDisallowIncoming; extern uint256 const featureXRPFees; extern uint256 const featureAMM; -extern uint256 const featureNFTokenMintOffer; extern uint256 const fixUniversalNumber; extern uint256 const fixNonFungibleTokensV1_2; extern uint256 const fixNFTokenRemint; @@ -353,6 +352,7 @@ extern uint256 const featureXChainBridge; extern uint256 const fixDisallowIncomingV1; extern uint256 const featureDID; extern uint256 const fixFillOrKill; +extern uint256 const featureNFTokenMintOffer; } // namespace ripple From f9e4cdaea311b0c503686ee771b4bdb26a3478e1 Mon Sep 17 00:00:00 2001 From: tequ Date: Tue, 5 Dec 2023 23:50:24 +0900 Subject: [PATCH 05/13] Update NFToken_test.cpp --- src/test/app/NFToken_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 8ce463a2d31..3e2fbb2d4d1 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -6917,7 +6917,7 @@ class NFToken0_test : public beast::unit_test::suite static FeatureBitset const all{supported_amendments()}; static FeatureBitset const fixNFTDir{fixNFTokenDirV1}; - static std::array const feats{ + static std::array const feats{ all - fixNFTDir - fixNonFungibleTokensV1_2 - fixNFTokenRemint - featureNFTokenMintOffer, all - disallowIncoming - fixNonFungibleTokensV1_2 - From a7f374ee49cb44b880889991fc5d7454a5f0ba45 Mon Sep 17 00:00:00 2001 From: tequ Date: Wed, 6 Dec 2023 00:39:03 +0900 Subject: [PATCH 06/13] fix sfAmount field --- src/ripple/app/tx/impl/NFTokenMint.cpp | 27 +++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/ripple/app/tx/impl/NFTokenMint.cpp b/src/ripple/app/tx/impl/NFTokenMint.cpp index 218df865482..dbcb04ec8dd 100644 --- a/src/ripple/app/tx/impl/NFTokenMint.cpp +++ b/src/ripple/app/tx/impl/NFTokenMint.cpp @@ -38,7 +38,8 @@ NFTokenMint::preflight(PreflightContext const& ctx) return temDISABLED; if (!ctx.rules.enabled(featureNFTokenMintOffer) && - (ctx.tx[sfAmount] || ctx.tx[~sfDestination] || ctx.tx[~sfExpiration])) + (ctx.tx.isFieldPresent(sfAmount) || ctx.tx.isFieldPresent(sfDestination) || + ctx.tx.isFieldPresent(sfExpiration))) return temDISABLED; if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) @@ -85,24 +86,24 @@ NFTokenMint::preflight(PreflightContext const& ctx) } // Amount field should be set if Destination and/or Expiration is set - if (ctx.tx[~sfDestination] || ctx.tx[~sfExpiration]) + if (ctx.tx.isFieldPresent(sfDestination) || ctx.tx.isFieldPresent(sfExpiration)) { if (!ctx.tx.isFieldPresent(sfAmount)) return temMALFORMED; } - if (STAmount const amount = ctx.tx[sfAmount]) + if (auto const amount = ctx.tx[~sfAmount]) { - if (amount.negative() && ctx.rules.enabled(fixNFTokenNegOffer)) + if (amount->negative() && ctx.rules.enabled(fixNFTokenNegOffer)) // An offer for a negative amount makes no sense. return temBAD_AMOUNT; - if (!isXRP(amount)) + if (!isXRP(*amount)) { if (ctx.tx.isFlag(tfOnlyXRP)) return temBAD_AMOUNT; - if (!amount) + if (!*amount) return temBAD_AMOUNT; } } @@ -173,35 +174,35 @@ NFTokenMint::preclaim(PreclaimContext const& ctx) return tecNO_PERMISSION; } - if (STAmount const amount = ctx.tx[sfAmount]) + if (auto const amount = ctx.tx[~sfAmount]) { if (hasExpired(ctx.view, ctx.tx[~sfExpiration])) return tecEXPIRED; if (!(ctx.tx.getFlags() & nft::flagCreateTrustLines) && - !amount.native() && ctx.tx[~sfTransferFee]) + !amount->native() && ctx.tx[~sfTransferFee]) { auto const nftIssuer = ctx.tx[~sfIssuer].value_or(ctx.tx[sfAccount]); if (!ctx.view.exists(keylet::account(nftIssuer))) return tecNO_ISSUER; - if (!ctx.view.exists(keylet::line(nftIssuer, amount.issue()))) + if (!ctx.view.exists(keylet::line(nftIssuer, amount->issue()))) return tecNO_LINE; if (isFrozen( ctx.view, nftIssuer, - amount.getCurrency(), - amount.getIssuer())) + amount->getCurrency(), + amount->getIssuer())) return tecFROZEN; } if (isFrozen( ctx.view, ctx.tx[sfAccount], - amount.getCurrency(), - amount.getIssuer())) + amount->getCurrency(), + amount->getIssuer())) return tecFROZEN; if (auto const destination = ctx.tx[~sfDestination]) From b8954f4b1ef1f1cd87dd684838e184462527b636 Mon Sep 17 00:00:00 2001 From: tequ Date: Wed, 31 Jan 2024 22:43:30 +0900 Subject: [PATCH 07/13] fix expiration, reserve, offer_id meta --- src/ripple/app/tx/impl/NFTokenMint.cpp | 38 ++++-- src/ripple/protocol/impl/NFTokenOfferID.cpp | 3 +- src/test/app/NFToken_test.cpp | 131 ++++++++++++++++---- 3 files changed, 132 insertions(+), 40 deletions(-) diff --git a/src/ripple/app/tx/impl/NFTokenMint.cpp b/src/ripple/app/tx/impl/NFTokenMint.cpp index dbcb04ec8dd..153185ed657 100644 --- a/src/ripple/app/tx/impl/NFTokenMint.cpp +++ b/src/ripple/app/tx/impl/NFTokenMint.cpp @@ -38,7 +38,8 @@ NFTokenMint::preflight(PreflightContext const& ctx) return temDISABLED; if (!ctx.rules.enabled(featureNFTokenMintOffer) && - (ctx.tx.isFieldPresent(sfAmount) || ctx.tx.isFieldPresent(sfDestination) || + (ctx.tx.isFieldPresent(sfAmount) || + ctx.tx.isFieldPresent(sfDestination) || ctx.tx.isFieldPresent(sfExpiration))) return temDISABLED; @@ -86,7 +87,8 @@ NFTokenMint::preflight(PreflightContext const& ctx) } // Amount field should be set if Destination and/or Expiration is set - if (ctx.tx.isFieldPresent(sfDestination) || ctx.tx.isFieldPresent(sfExpiration)) + if (ctx.tx.isFieldPresent(sfDestination) || + ctx.tx.isFieldPresent(sfExpiration)) { if (!ctx.tx.isFieldPresent(sfAmount)) return temMALFORMED; @@ -108,6 +110,16 @@ NFTokenMint::preflight(PreflightContext const& ctx) } } + if (auto exp = ctx.tx[~sfExpiration]; exp == 0) + return temBAD_EXPIRATION; + + if (auto dest = ctx.tx[~sfDestination]) + { + // The destination can't be the account executing the transaction. + if (dest == ctx.tx[sfAccount]) + return temMALFORMED; + } + return preflight2(ctx); } @@ -344,7 +356,7 @@ NFTokenMint::doApply() // Create the offer: { // Token offers are always added to the owner's owner directory: - auto const ownerNode = view().dirInsert( + auto const ownerNode = ctx_.view().dirInsert( keylet::ownerDir(account_), offerID, describeOwnerDir(account_)); @@ -352,11 +364,9 @@ NFTokenMint::doApply() if (!ownerNode) return tecDIR_FULL; - bool const isSellOffer = true; - - // Token offers are also added to the token's buy or sell offer + // Token offers are also added to the token's sell offer // directory - auto const offerNode = view().dirInsert( + auto const offerNode = ctx_.view().dirInsert( keylet::nft_sells(nftokenID), offerID, [&nftokenID](std::shared_ptr const& sle) { @@ -367,10 +377,8 @@ NFTokenMint::doApply() if (!offerNode) return tecDIR_FULL; - std::uint32_t sleFlags = 0; - - if (isSellOffer) - sleFlags |= lsfSellNFToken; + // Only sell offer are supported at NFTokenMint. + std::uint32_t sleFlags = lsfSellNFToken; auto offer = std::make_shared(offerID); (*offer)[sfOwner] = account_; @@ -386,7 +394,13 @@ NFTokenMint::doApply() if (auto const destination = ctx_.tx[~sfDestination]) (*offer)[sfDestination] = *destination; - view().insert(offer); + ctx_.view().insert(offer); + + adjustOwnerCount( + ctx_.view(), + ctx_.view().peek(keylet::account(account_)), + 1, + j_); } } diff --git a/src/ripple/protocol/impl/NFTokenOfferID.cpp b/src/ripple/protocol/impl/NFTokenOfferID.cpp index c9c3118cf2a..51408bfb21a 100644 --- a/src/ripple/protocol/impl/NFTokenOfferID.cpp +++ b/src/ripple/protocol/impl/NFTokenOfferID.cpp @@ -31,7 +31,8 @@ canHaveNFTokenOfferID( return false; TxType const tt = serializedTx->getTxnType(); - if (tt != ttNFTOKEN_CREATE_OFFER) + if (!(tt == ttNFTOKEN_MINT && serializedTx->isFieldPresent(sfAmount)) && + tt != ttNFTOKEN_CREATE_OFFER) return false; // if the transaction failed nothing could have been delivered. diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 08a57e325af..f65ddbe209d 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -6567,9 +6567,9 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite } void - testFeatNFTokenMintAndOffer(FeatureBitset features) + testFeatMintWithOffer(FeatureBitset features) { - testcase("NFTokenMint and Create NFTokenOffer"); + testcase("NFTokenMint with Create NFTokenOffer"); using namespace test::jtx; @@ -6591,53 +6591,111 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite { Env env{*this, features}; Account const alice("alice"); + Account const buyer{"buyer"}; Account const becky("becky"); - env.fund(XRP(10000), alice, becky); + env.fund(XRP(10000), alice, buyer); env.close(); + { // Destination field specified but Amount field not specified env(token::mint(alice), - token::destination(becky), + token::destination(buyer), ter(temMALFORMED)); env.close(); + BEAST_EXPECT(ownerCount(env, alice) == 0); // Expiration field specified but Amount field not specified env(token::mint(alice), token::expiration(lastClose(env) + 25), ter(temMALFORMED)); env.close(); + BEAST_EXPECT(ownerCount(env, buyer) == 0); + } - // Amount field specified - BEAST_EXPECT(ownerCount(env, alice) == 1); - env(token::mint(alice), token::amount(XRP(10))); - BEAST_EXPECT(ownerCount(env, alice) == 2); + { + // The destination may not be the account submitting the + // transaction. + env(token::mint(alice), + token::amount(XRP(1000)), + token::destination(alice), + ter(temMALFORMED)); env.close(); + BEAST_EXPECT(ownerCount(env, alice) == 0); - // Amount field and Destination field, Expiration field specified + // The destination must be an account already established in the + // ledger. env(token::mint(alice), - token::amount(XRP(10)), - token::destination(becky), - token::expiration(lastClose(env) + 25)); + token::amount(XRP(1000)), + token::destination(Account("demon")), + ter(tecNO_DST)); env.close(); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + { + // Set a bad expiration. env(token::mint(alice), - token::amount(XRP(10)), - token::destination(becky), - token::expiration(lastClose(env) + 25)); - uint256 const offerAliceSellsToBecky = - keylet::nftoffer(alice, env.seq(alice)).key; - env(token::cancelOffer(alice, {offerAliceSellsToBecky})); + token::amount(XRP(1000)), + token::expiration(0), + ter(temBAD_EXPIRATION)); env.close(); + BEAST_EXPECT(ownerCount(env, alice) == 0); - env(token::mint(becky), - token::amount(XRP(10)), - token::destination(alice), - token::expiration(lastClose(env) + 25)); - uint256 const offerBeckySellsToAlice = - keylet::nftoffer(becky, env.seq(becky)).key; - env(token::cancelOffer(alice, {offerBeckySellsToAlice})); + // The new NFTokenOffer may not have passed its expiration time. + env(token::mint(alice), + token::amount(XRP(1000)), + token::expiration(lastClose(env)), + ter(tecEXPIRED)); + env.close(); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + { + // Set an invalid amount. + env(token::mint(alice), + token::amount(buyer["USD"](1)), + txflags(tfOnlyXRP), + ter(temBAD_AMOUNT)); + env(token::mint(alice), + token::amount(buyer["USD"](0)), + ter(temBAD_AMOUNT)); env.close(); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + // Amount field specified + BEAST_EXPECT(ownerCount(env, alice) == 0); + env(token::mint(alice), token::amount(XRP(10))); + BEAST_EXPECT(ownerCount(env, alice) == 2); + env.close(); + + // Amount field and Destination field, Expiration field specified + env(token::mint(alice), + token::amount(XRP(10)), + token::destination(buyer), + token::expiration(lastClose(env) + 25)); + env.close(); + + // Can be canceled by the issuer. + env(token::mint(alice), + token::amount(XRP(10)), + token::destination(buyer), + token::expiration(lastClose(env) + 25)); + uint256 const offerAliceSellsToBuyer = + keylet::nftoffer(alice, env.seq(alice)).key; + env(token::cancelOffer(alice, {offerAliceSellsToBuyer})); + env.close(); + + // Can be canceled by the buyer. + env(token::mint(buyer), + token::amount(XRP(10)), + token::destination(alice), + token::expiration(lastClose(env) + 25)); + uint256 const offerBuyerSellsToAlice = + keylet::nftoffer(buyer, env.seq(buyer)).key; + env(token::cancelOffer(alice, {offerBuyerSellsToAlice})); + env.close(); } } @@ -6871,6 +6929,15 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite env.close(); verifyNFTokenIDsInCancelOffer({nftId}); } + + if (features[featureNFTokenMintOffer]) { + uint256 const aliceMintWithOfferIndex1 = + keylet::nftoffer(alice, env.seq(alice)).key; + env(token::mint(alice), + token::amount(XRP(0))); + env.close(); + verifyNFTokenOfferID(aliceMintWithOfferIndex1); + } } void @@ -6905,8 +6972,8 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite testIOUWithTransferFee(features); testBrokeredSaleToSelf(features); testFixNFTokenRemint(features); + testFeatMintWithOffer(features); testTxJsonMetaFields(features); - testFeatNFTokenMintAndOffer(features); } public: @@ -6969,12 +7036,21 @@ class NFTokenWOTokenRemint_test : public NFTokenBaseUtil_test } }; +class NFTokenWOMintOffer_test : public NFTokenBaseUtil_test +{ + void + run() override + { + NFTokenBaseUtil_test::run(4); + } +}; + class NFTokenAllFeatures_test : public NFTokenBaseUtil_test { void run() override { - NFTokenBaseUtil_test::run(4, true); + NFTokenBaseUtil_test::run(5, true); } }; @@ -6982,6 +7058,7 @@ BEAST_DEFINE_TESTSUITE_PRIO(NFTokenBaseUtil, tx, ripple, 2); BEAST_DEFINE_TESTSUITE_PRIO(NFTokenDisallowIncoming, tx, ripple, 2); BEAST_DEFINE_TESTSUITE_PRIO(NFTokenWOfixV1, tx, ripple, 2); BEAST_DEFINE_TESTSUITE_PRIO(NFTokenWOTokenRemint, tx, ripple, 2); +BEAST_DEFINE_TESTSUITE_PRIO(NFTokenWOMintOffer, tx, ripple, 2); BEAST_DEFINE_TESTSUITE_PRIO(NFTokenAllFeatures, tx, ripple, 2); } // namespace ripple From e48768dec0ea5a0ae404c61a62e74e2a2d867b44 Mon Sep 17 00:00:00 2001 From: tequ Date: Wed, 31 Jan 2024 22:55:06 +0900 Subject: [PATCH 08/13] clang-format --- src/test/app/NFToken_test.cpp | 180 +++++++++++++++++----------------- 1 file changed, 90 insertions(+), 90 deletions(-) diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index f65ddbe209d..e76ba9873a5 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -6597,105 +6597,105 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite env.fund(XRP(10000), alice, buyer); env.close(); - { - // Destination field specified but Amount field not specified - env(token::mint(alice), - token::destination(buyer), - ter(temMALFORMED)); - env.close(); - BEAST_EXPECT(ownerCount(env, alice) == 0); + { + // Destination field specified but Amount field not specified + env(token::mint(alice), + token::destination(buyer), + ter(temMALFORMED)); + env.close(); + BEAST_EXPECT(ownerCount(env, alice) == 0); - // Expiration field specified but Amount field not specified - env(token::mint(alice), - token::expiration(lastClose(env) + 25), - ter(temMALFORMED)); - env.close(); - BEAST_EXPECT(ownerCount(env, buyer) == 0); - } + // Expiration field specified but Amount field not specified + env(token::mint(alice), + token::expiration(lastClose(env) + 25), + ter(temMALFORMED)); + env.close(); + BEAST_EXPECT(ownerCount(env, buyer) == 0); + } - { - // The destination may not be the account submitting the - // transaction. - env(token::mint(alice), - token::amount(XRP(1000)), - token::destination(alice), - ter(temMALFORMED)); - env.close(); - BEAST_EXPECT(ownerCount(env, alice) == 0); + { + // The destination may not be the account submitting the + // transaction. + env(token::mint(alice), + token::amount(XRP(1000)), + token::destination(alice), + ter(temMALFORMED)); + env.close(); + BEAST_EXPECT(ownerCount(env, alice) == 0); + + // The destination must be an account already established in the + // ledger. + env(token::mint(alice), + token::amount(XRP(1000)), + token::destination(Account("demon")), + ter(tecNO_DST)); + env.close(); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } - // The destination must be an account already established in the - // ledger. - env(token::mint(alice), - token::amount(XRP(1000)), - token::destination(Account("demon")), - ter(tecNO_DST)); - env.close(); + { + // Set a bad expiration. + env(token::mint(alice), + token::amount(XRP(1000)), + token::expiration(0), + ter(temBAD_EXPIRATION)); + env.close(); + BEAST_EXPECT(ownerCount(env, alice) == 0); + + // The new NFTokenOffer may not have passed its expiration time. + env(token::mint(alice), + token::amount(XRP(1000)), + token::expiration(lastClose(env)), + ter(tecEXPIRED)); + env.close(); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + { + // Set an invalid amount. + env(token::mint(alice), + token::amount(buyer["USD"](1)), + txflags(tfOnlyXRP), + ter(temBAD_AMOUNT)); + env(token::mint(alice), + token::amount(buyer["USD"](0)), + ter(temBAD_AMOUNT)); + env.close(); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + // Amount field specified BEAST_EXPECT(ownerCount(env, alice) == 0); - } + env(token::mint(alice), token::amount(XRP(10))); + BEAST_EXPECT(ownerCount(env, alice) == 2); + env.close(); - { - // Set a bad expiration. + // Amount field and Destination field, Expiration field specified env(token::mint(alice), - token::amount(XRP(1000)), - token::expiration(0), - ter(temBAD_EXPIRATION)); + token::amount(XRP(10)), + token::destination(buyer), + token::expiration(lastClose(env) + 25)); env.close(); - BEAST_EXPECT(ownerCount(env, alice) == 0); - // The new NFTokenOffer may not have passed its expiration time. + // Can be canceled by the issuer. env(token::mint(alice), - token::amount(XRP(1000)), - token::expiration(lastClose(env)), - ter(tecEXPIRED)); + token::amount(XRP(10)), + token::destination(buyer), + token::expiration(lastClose(env) + 25)); + uint256 const offerAliceSellsToBuyer = + keylet::nftoffer(alice, env.seq(alice)).key; + env(token::cancelOffer(alice, {offerAliceSellsToBuyer})); env.close(); - BEAST_EXPECT(ownerCount(env, alice) == 0); - } - { - // Set an invalid amount. - env(token::mint(alice), - token::amount(buyer["USD"](1)), - txflags(tfOnlyXRP), - ter(temBAD_AMOUNT)); - env(token::mint(alice), - token::amount(buyer["USD"](0)), - ter(temBAD_AMOUNT)); + // Can be canceled by the buyer. + env(token::mint(buyer), + token::amount(XRP(10)), + token::destination(alice), + token::expiration(lastClose(env) + 25)); + uint256 const offerBuyerSellsToAlice = + keylet::nftoffer(buyer, env.seq(buyer)).key; + env(token::cancelOffer(alice, {offerBuyerSellsToAlice})); env.close(); - BEAST_EXPECT(ownerCount(env, alice) == 0); - } - - // Amount field specified - BEAST_EXPECT(ownerCount(env, alice) == 0); - env(token::mint(alice), token::amount(XRP(10))); - BEAST_EXPECT(ownerCount(env, alice) == 2); - env.close(); - - // Amount field and Destination field, Expiration field specified - env(token::mint(alice), - token::amount(XRP(10)), - token::destination(buyer), - token::expiration(lastClose(env) + 25)); - env.close(); - - // Can be canceled by the issuer. - env(token::mint(alice), - token::amount(XRP(10)), - token::destination(buyer), - token::expiration(lastClose(env) + 25)); - uint256 const offerAliceSellsToBuyer = - keylet::nftoffer(alice, env.seq(alice)).key; - env(token::cancelOffer(alice, {offerAliceSellsToBuyer})); - env.close(); - - // Can be canceled by the buyer. - env(token::mint(buyer), - token::amount(XRP(10)), - token::destination(alice), - token::expiration(lastClose(env) + 25)); - uint256 const offerBuyerSellsToAlice = - keylet::nftoffer(buyer, env.seq(buyer)).key; - env(token::cancelOffer(alice, {offerBuyerSellsToAlice})); - env.close(); } } @@ -6930,11 +6930,11 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite verifyNFTokenIDsInCancelOffer({nftId}); } - if (features[featureNFTokenMintOffer]) { + if (features[featureNFTokenMintOffer]) + { uint256 const aliceMintWithOfferIndex1 = keylet::nftoffer(alice, env.seq(alice)).key; - env(token::mint(alice), - token::amount(XRP(0))); + env(token::mint(alice), token::amount(XRP(0))); env.close(); verifyNFTokenOfferID(aliceMintWithOfferIndex1); } From 65d62df17f2193baace12a61b1ca7db2373c7e29 Mon Sep 17 00:00:00 2001 From: tequ Date: Thu, 1 Feb 2024 14:52:40 +0900 Subject: [PATCH 09/13] Add test for trustline, disallowincoming --- src/test/app/NFToken_test.cpp | 124 ++++++++++++++++++++++++++++++++-- 1 file changed, 120 insertions(+), 4 deletions(-) diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index e76ba9873a5..e7adfa8bb0a 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -3171,6 +3171,25 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite ter(tecNO_PERMISSION)); env.close(); } + + // minter mint and offer to buyer + if (features[featureNFTokenMintOffer]) + { + // a sell offer from the minter to the buyer should be rejected + env(token::mint(minter), + token::amount(drops(1)), + token::destination(buyer), + ter(tecNO_PERMISSION)); + env.close(); + } + else + { + env(token::mint(minter), + token::amount(drops(1)), + token::destination(buyer), + ter(temDISABLED)); + env.close(); + } } void @@ -6577,14 +6596,25 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite { Env env{*this, features}; Account const alice("alice"); - Account const becky("becky"); + Account const buyer("buyer"); - env.fund(XRP(10000), alice, becky); + env.fund(XRP(10000), alice, buyer); env.close(); env(token::mint(alice), token::amount(XRP(10000)), ter(temDISABLED)); + env.close(); + + env(token::mint(alice), + token::destination("buyer"), + ter(temDISABLED)); + env.close(); + + env(token::mint(alice), + token::expiration(lastClose(env) + 25), + ter(temDISABLED)); + env.close(); } if (features[featureNFTokenMintOffer]) @@ -6592,9 +6622,12 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite Env env{*this, features}; Account const alice("alice"); Account const buyer{"buyer"}; - Account const becky("becky"); + Account const gw("gw"); + Account const issuer("issuer"); + Account const minter("minter"); + IOU const gwAUD(gw["AUD"]); - env.fund(XRP(10000), alice, buyer); + env.fund(XRP(10000), alice, buyer, gw, issuer, minter); env.close(); { @@ -6662,6 +6695,40 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite ter(temBAD_AMOUNT)); env.close(); BEAST_EXPECT(ownerCount(env, alice) == 0); + + // Issuer (alice) must have a trust line for the offered funds. + env(token::mint(alice), + token::amount(gwAUD(1000)), + txflags(tfTransferable), + token::xferFee(10), + ter(tecNO_LINE)); + env.close(); + BEAST_EXPECT(ownerCount(env, alice) == 0); + + // Give alice the needed trust line, but freeze it. + env(trust(gw, alice["AUD"](999), tfSetFreeze)); + env.close(); + + // Issuer (alice) must have a trust line for the offered funds + // and the trust line may not be frozen. + env(token::mint(alice), + token::amount(gwAUD(1000)), + txflags(tfTransferable), + token::xferFee(10), + ter(tecFROZEN)); + env.close(); + BEAST_EXPECT(ownerCount(env, alice) == 0); + + // Seller (alice) must have a trust line may not be frozen. + env(token::mint(alice), + token::amount(gwAUD(1000)), + ter(tecFROZEN)); + env.close(); + BEAST_EXPECT(ownerCount(env, alice) == 0); + + // Unfreeze alice's trustline. + env(trust(gw, alice["AUD"](999), tfClearFreeze)); + env.close(); } // Amount field specified @@ -6677,6 +6744,17 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite token::expiration(lastClose(env) + 25)); env.close(); + // With TransferFee field + env(trust(alice, gwAUD(1000))); + env.close(); + env(token::mint(alice), + token::amount(gwAUD(1)), + token::destination(buyer), + token::expiration(lastClose(env) + 25), + txflags(tfTransferable), + token::xferFee(10)); + env.close(); + // Can be canceled by the issuer. env(token::mint(alice), token::amount(XRP(10)), @@ -6696,6 +6774,44 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite keylet::nftoffer(buyer, env.seq(buyer)).key; env(token::cancelOffer(alice, {offerBuyerSellsToAlice})); env.close(); + + env(token::setMinter(issuer, minter)); + env.close(); + + // Minter will have offer not issuer + BEAST_EXPECT(ownerCount(env, minter) == 0); + BEAST_EXPECT(ownerCount(env, issuer) == 0); + env(token::mint(minter), + token::issuer(issuer), + token::amount(drops(1))); + env.close(); + BEAST_EXPECT(ownerCount(env, minter) == 2); + BEAST_EXPECT(ownerCount(env, issuer) == 0); + } + + // Test sell offers with a destination with and without + // fixNFTokenNegOffer. + if (features[featureNFTokenMintOffer]) + { + for (auto const& tweakedFeatures : + {features - fixNFTokenNegOffer - featureNonFungibleTokensV1_1, + features | fixNFTokenNegOffer}) + { + Env env{*this, tweakedFeatures}; + Account const alice("alice"); + + env.fund(XRP(1000000), alice); + + TER const offerCreateTER = tweakedFeatures[fixNFTokenNegOffer] + ? static_cast(temBAD_AMOUNT) + : static_cast(tesSUCCESS); + + // Make offers with negative amounts for the NFTs + env(token::mint(alice), + token::amount(XRP(-2)), + ter(offerCreateTER)); + env.close(); + } } } From 95b2e08fce19bb6416d63456769fd58b62b7916e Mon Sep 17 00:00:00 2001 From: tequ Date: Thu, 1 Feb 2024 18:35:16 +0900 Subject: [PATCH 10/13] fix duplicate error handling --- src/ripple/app/tx/impl/NFTokenMint.cpp | 2 -- src/test/app/NFToken_test.cpp | 11 ++++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/ripple/app/tx/impl/NFTokenMint.cpp b/src/ripple/app/tx/impl/NFTokenMint.cpp index 153185ed657..82b06e7d69c 100644 --- a/src/ripple/app/tx/impl/NFTokenMint.cpp +++ b/src/ripple/app/tx/impl/NFTokenMint.cpp @@ -196,8 +196,6 @@ NFTokenMint::preclaim(PreclaimContext const& ctx) { auto const nftIssuer = ctx.tx[~sfIssuer].value_or(ctx.tx[sfAccount]); - if (!ctx.view.exists(keylet::account(nftIssuer))) - return tecNO_ISSUER; if (!ctx.view.exists(keylet::line(nftIssuer, amount->issue()))) return tecNO_LINE; diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index e7adfa8bb0a..d5a56bfa608 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -3175,19 +3175,20 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite // minter mint and offer to buyer if (features[featureNFTokenMintOffer]) { + // enable flag + env(fset(buyer, asfDisallowIncomingNFTokenOffer)); // a sell offer from the minter to the buyer should be rejected env(token::mint(minter), token::amount(drops(1)), token::destination(buyer), ter(tecNO_PERMISSION)); env.close(); - } - else - { + + // disable flag + env(fclear(buyer, asfDisallowIncomingNFTokenOffer)); env(token::mint(minter), token::amount(drops(1)), - token::destination(buyer), - ter(temDISABLED)); + token::destination(buyer)); env.close(); } } From 6e0d26ff58395a50b5c89335da67d71c010cdd72 Mon Sep 17 00:00:00 2001 From: tequ Date: Thu, 14 Mar 2024 16:56:10 +0900 Subject: [PATCH 11/13] Addressing Reviews --- src/ripple/app/tx/impl/NFTokenMint.cpp | 6 ++--- src/test/app/NFToken_test.cpp | 37 ++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/src/ripple/app/tx/impl/NFTokenMint.cpp b/src/ripple/app/tx/impl/NFTokenMint.cpp index 82b06e7d69c..c8e8a3b0512 100644 --- a/src/ripple/app/tx/impl/NFTokenMint.cpp +++ b/src/ripple/app/tx/impl/NFTokenMint.cpp @@ -110,10 +110,10 @@ NFTokenMint::preflight(PreflightContext const& ctx) } } - if (auto exp = ctx.tx[~sfExpiration]; exp == 0) + if (auto const exp = ctx.tx[~sfExpiration]; exp == 0) return temBAD_EXPIRATION; - if (auto dest = ctx.tx[~sfDestination]) + if (auto const dest = ctx.tx[~sfDestination]) { // The destination can't be the account executing the transaction. if (dest == ctx.tx[sfAccount]) @@ -376,7 +376,7 @@ NFTokenMint::doApply() return tecDIR_FULL; // Only sell offer are supported at NFTokenMint. - std::uint32_t sleFlags = lsfSellNFToken; + std::uint32_t const sleFlags = lsfSellNFToken; auto offer = std::make_shared(offerID); (*offer)[sfOwner] = account_; diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index eb187401df8..c3102f66b03 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -6626,6 +6626,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite Account const gw("gw"); Account const issuer("issuer"); Account const minter("minter"); + Account const bob("bob"); IOU const gwAUD(gw["AUD"]); env.fund(XRP(10000), alice, buyer, gw, issuer, minter); @@ -6732,6 +6733,42 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite env.close(); } + { + // check reserve + auto const acctReserve = + env.current()->fees().accountReserve(0); + auto const incReserve = env.current()->fees().increment; + + env.fund(acctReserve + incReserve, bob); + env.close(); + + // doesn't have reserve for 2 objects (NFTokenPage, Offer) + env(token::mint(bob), + token::amount(XRP(0)), + ter(tecINSUFFICIENT_RESERVE)); + env.close(); + + // have reserve for NFTokenPage, Offer + env(pay(env.master, bob, incReserve + drops(10))); + env.close(); + env(token::mint(bob), token::amount(XRP(0))); + env.close(); + + // doesn't have reserve for Offer + env(pay(env.master, bob, drops(10))); + env.close(); + env(token::mint(bob), + token::amount(XRP(0)), + ter(tecINSUFFICIENT_RESERVE)); + env.close(); + + // have reserve for Offer + env(pay(env.master, bob, incReserve + drops(10))); + env.close(); + env(token::mint(bob), token::amount(XRP(0))); + env.close(); + } + // Amount field specified BEAST_EXPECT(ownerCount(env, alice) == 0); env(token::mint(alice), token::amount(XRP(10))); From f65eea7b220bdab1b419666badb62e6122e99561 Mon Sep 17 00:00:00 2001 From: tequ Date: Fri, 15 Mar 2024 12:32:05 +0900 Subject: [PATCH 12/13] fix same as fixNFTokenTrustlineSurprise --- src/ripple/app/tx/impl/NFTokenMint.cpp | 5 ++++- src/test/app/NFToken_test.cpp | 9 +++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/ripple/app/tx/impl/NFTokenMint.cpp b/src/ripple/app/tx/impl/NFTokenMint.cpp index c8e8a3b0512..aa1588baa95 100644 --- a/src/ripple/app/tx/impl/NFTokenMint.cpp +++ b/src/ripple/app/tx/impl/NFTokenMint.cpp @@ -197,7 +197,10 @@ NFTokenMint::preclaim(PreclaimContext const& ctx) auto const nftIssuer = ctx.tx[~sfIssuer].value_or(ctx.tx[sfAccount]); - if (!ctx.view.exists(keylet::line(nftIssuer, amount->issue()))) + // If the IOU issuer and the NFToken issuer are the same, + // then that issuer does not need a trust line to accept their fee. + if (nftIssuer != amount->getIssuer() && + !ctx.view.read(keylet::line(nftIssuer, amount->issue()))) return tecNO_LINE; if (isFrozen( diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index c3102f66b03..f6277a61014 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -6707,6 +6707,15 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite env.close(); BEAST_EXPECT(ownerCount(env, alice) == 0); + // If the IOU issuer and the NFToken issuer are the same, + // then that issuer does not need a trust line to accept their + // fee. + env(token::mint(gw), + token::amount(gwAUD(1000)), + txflags(tfTransferable), + token::xferFee(10)); + env.close(); + // Give alice the needed trust line, but freeze it. env(trust(gw, alice["AUD"](999), tfSetFreeze)); env.close(); From fe4d616776559d33a53e22d6607c5f98636f347f Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Mon, 29 Apr 2024 14:47:02 -0700 Subject: [PATCH 13/13] [FOLD] Refactor NFTokenMint and NFTokenCreateOffer to share code --- src/ripple/app/tx/impl/NFTokenCreateOffer.cpp | 261 +++--------------- src/ripple/app/tx/impl/NFTokenMint.cpp | 183 ++++-------- .../app/tx/impl/details/NFTokenUtils.cpp | 247 +++++++++++++++++ src/ripple/app/tx/impl/details/NFTokenUtils.h | 41 +++ src/test/app/NFToken_test.cpp | 194 +++++++++++-- 5 files changed, 561 insertions(+), 365 deletions(-) diff --git a/src/ripple/app/tx/impl/NFTokenCreateOffer.cpp b/src/ripple/app/tx/impl/NFTokenCreateOffer.cpp index 22eca2dffdd..e1a0e04aa52 100644 --- a/src/ripple/app/tx/impl/NFTokenCreateOffer.cpp +++ b/src/ripple/app/tx/impl/NFTokenCreateOffer.cpp @@ -37,65 +37,24 @@ NFTokenCreateOffer::preflight(PreflightContext const& ctx) return ret; auto const txFlags = ctx.tx.getFlags(); - bool const isSellOffer = txFlags & tfSellNFToken; if (txFlags & tfNFTokenCreateOfferMask) return temINVALID_FLAG; - auto const account = ctx.tx[sfAccount]; auto const nftFlags = nft::getFlags(ctx.tx[sfNFTokenID]); - { - STAmount const amount = ctx.tx[sfAmount]; - - if (amount.negative() && ctx.rules.enabled(fixNFTokenNegOffer)) - // An offer for a negative amount makes no sense. - return temBAD_AMOUNT; - - if (!isXRP(amount)) - { - if (nftFlags & nft::flagOnlyXRP) - return temBAD_AMOUNT; - - if (!amount) - return temBAD_AMOUNT; - } - - // If this is an offer to buy, you must offer something; if it's an - // offer to sell, you can ask for nothing. - if (!isSellOffer && !amount) - return temBAD_AMOUNT; - } - - if (auto exp = ctx.tx[~sfExpiration]; exp == 0) - return temBAD_EXPIRATION; - - auto const owner = ctx.tx[~sfOwner]; - - // The 'Owner' field must be present when offering to buy, but can't - // be present when selling (it's implicit): - if (owner.has_value() == isSellOffer) - return temMALFORMED; - - if (owner && owner == account) - return temMALFORMED; - - if (auto dest = ctx.tx[~sfDestination]) - { - // Some folks think it makes sense for a buy offer to specify a - // specific broker using the Destination field. This change doesn't - // deserve it's own amendment, so we're piggy-backing on - // fixNFTokenNegOffer. - // - // Prior to fixNFTokenNegOffer any use of the Destination field on - // a buy offer was malformed. - if (!isSellOffer && !ctx.rules.enabled(fixNFTokenNegOffer)) - return temMALFORMED; - - // The destination can't be the account executing the transaction. - if (dest == account) - return temMALFORMED; - } + // Use implementation shared with NFTokenMint + if (NotTEC notTec = nft::tokenOfferCreatePreflight( + ctx.tx[sfAccount], + ctx.tx[sfAmount], + ctx.tx[~sfDestination], + ctx.tx[~sfExpiration], + nftFlags, + ctx.rules, + ctx.tx[~sfOwner], + txFlags); + !isTesSuccess(notTec)) + return notTec; return preflight2(ctx); } @@ -106,182 +65,44 @@ NFTokenCreateOffer::preclaim(PreclaimContext const& ctx) if (hasExpired(ctx.view, ctx.tx[~sfExpiration])) return tecEXPIRED; - auto const nftokenID = ctx.tx[sfNFTokenID]; - bool const isSellOffer = ctx.tx.isFlag(tfSellNFToken); + uint256 const nftokenID = ctx.tx[sfNFTokenID]; + std::uint32_t const txFlags = {ctx.tx.getFlags()}; if (!nft::findToken( - ctx.view, ctx.tx[isSellOffer ? sfAccount : sfOwner], nftokenID)) - return tecNO_ENTRY; - - auto const nftFlags = nft::getFlags(nftokenID); - auto const issuer = nft::getIssuer(nftokenID); - auto const amount = ctx.tx[sfAmount]; - - if (!(nftFlags & nft::flagCreateTrustLines) && !amount.native() && - nft::getTransferFee(nftokenID)) - { - if (!ctx.view.exists(keylet::account(issuer))) - return tecNO_ISSUER; - - if (!ctx.view.exists(keylet::line(issuer, amount.issue()))) - return tecNO_LINE; - - if (isFrozen( - ctx.view, issuer, amount.getCurrency(), amount.getIssuer())) - return tecFROZEN; - } - - if (issuer != ctx.tx[sfAccount] && !(nftFlags & nft::flagTransferable)) - { - auto const root = ctx.view.read(keylet::account(issuer)); - assert(root); - - if (auto minter = (*root)[~sfNFTokenMinter]; - minter != ctx.tx[sfAccount]) - return tefNFTOKEN_IS_NOT_TRANSFERABLE; - } - - if (isFrozen( ctx.view, - ctx.tx[sfAccount], - amount.getCurrency(), - amount.getIssuer())) - return tecFROZEN; - - // If this is an offer to buy the token, the account must have the - // needed funds at hand; but note that funds aren't reserved and the - // offer may later become unfunded. - if (!isSellOffer) - { - // After this amendment, we allow an IOU issuer to make a buy offer - // using their own currency. - if (ctx.view.rules().enabled(fixNonFungibleTokensV1_2)) - { - 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; - } - - if (auto const destination = ctx.tx[~sfDestination]) - { - // If a destination is specified, the destination must already be in - // the ledger. - auto const sleDst = ctx.view.read(keylet::account(*destination)); - - if (!sleDst) - return tecNO_DST; - - // check if the destination has disallowed incoming offers - if (ctx.view.rules().enabled(featureDisallowIncoming)) - { - // flag cannot be set unless amendment is enabled but - // out of an abundance of caution check anyway - - if (sleDst->getFlags() & lsfDisallowIncomingNFTokenOffer) - return tecNO_PERMISSION; - } - } - - if (auto const owner = ctx.tx[~sfOwner]) - { - // Check if the owner (buy offer) has disallowed incoming offers - if (ctx.view.rules().enabled(featureDisallowIncoming)) - { - auto const sleOwner = ctx.view.read(keylet::account(*owner)); - - // defensively check - // it should not be possible to specify owner that doesn't exist - if (!sleOwner) - return tecNO_TARGET; - - if (sleOwner->getFlags() & lsfDisallowIncomingNFTokenOffer) - return tecNO_PERMISSION; - } - } + ctx.tx[(txFlags & tfSellNFToken) ? sfAccount : sfOwner], + nftokenID)) + return tecNO_ENTRY; - return tesSUCCESS; + // Use implementation shared with NFTokenMint + return nft::tokenOfferCreatePreclaim( + ctx.view, + ctx.tx[sfAccount], + nft::getIssuer(nftokenID), + ctx.tx[sfAmount], + ctx.tx[~sfDestination], + nft::getFlags(nftokenID), + nft::getTransferFee(nftokenID), + ctx.j, + ctx.tx[~sfOwner], + txFlags); } TER NFTokenCreateOffer::doApply() { - if (auto const acct = view().read(keylet::account(ctx_.tx[sfAccount])); - mPriorBalance < view().fees().accountReserve((*acct)[sfOwnerCount] + 1)) - return tecINSUFFICIENT_RESERVE; - - auto const nftokenID = ctx_.tx[sfNFTokenID]; - - auto const offerID = - keylet::nftoffer(account_, ctx_.tx.getSeqProxy().value()); - - // Create the offer: - { - // Token offers are always added to the owner's owner directory: - auto const ownerNode = view().dirInsert( - keylet::ownerDir(account_), offerID, describeOwnerDir(account_)); - - if (!ownerNode) - return tecDIR_FULL; - - bool const isSellOffer = ctx_.tx.isFlag(tfSellNFToken); - - // Token offers are also added to the token's buy or sell offer - // directory - auto const offerNode = view().dirInsert( - isSellOffer ? keylet::nft_sells(nftokenID) - : keylet::nft_buys(nftokenID), - offerID, - [&nftokenID, isSellOffer](std::shared_ptr const& sle) { - (*sle)[sfFlags] = - isSellOffer ? lsfNFTokenSellOffers : lsfNFTokenBuyOffers; - (*sle)[sfNFTokenID] = nftokenID; - }); - - if (!offerNode) - return tecDIR_FULL; - - std::uint32_t sleFlags = 0; - - if (isSellOffer) - sleFlags |= lsfSellNFToken; - - auto offer = std::make_shared(offerID); - (*offer)[sfOwner] = account_; - (*offer)[sfNFTokenID] = nftokenID; - (*offer)[sfAmount] = ctx_.tx[sfAmount]; - (*offer)[sfFlags] = sleFlags; - (*offer)[sfOwnerNode] = *ownerNode; - (*offer)[sfNFTokenOfferNode] = *offerNode; - - if (auto const expiration = ctx_.tx[~sfExpiration]) - (*offer)[sfExpiration] = *expiration; - - if (auto const destination = ctx_.tx[~sfDestination]) - (*offer)[sfDestination] = *destination; - - view().insert(offer); - } - - // Update owner count. - adjustOwnerCount(view(), view().peek(keylet::account(account_)), 1, j_); - - return tesSUCCESS; + // Use implementation shared with NFTokenMint + return nft::tokenOfferCreateApply( + view(), + ctx_.tx[sfAccount], + ctx_.tx[sfAmount], + ctx_.tx[~sfDestination], + ctx_.tx[~sfExpiration], + ctx_.tx.getSeqProxy(), + ctx_.tx[sfNFTokenID], + mPriorBalance, + j_, + ctx_.tx.getFlags()); } } // namespace ripple diff --git a/src/ripple/app/tx/impl/NFTokenMint.cpp b/src/ripple/app/tx/impl/NFTokenMint.cpp index aa1588baa95..daffd29ed31 100644 --- a/src/ripple/app/tx/impl/NFTokenMint.cpp +++ b/src/ripple/app/tx/impl/NFTokenMint.cpp @@ -31,16 +31,23 @@ namespace ripple { +static std::uint16_t +extractNFTokenFlagsFromTxFlags(std::uint32_t txFlags) +{ + return static_cast(txFlags & 0x0000FFFF); +} + NotTEC NFTokenMint::preflight(PreflightContext const& ctx) { if (!ctx.rules.enabled(featureNonFungibleTokensV1)) return temDISABLED; - if (!ctx.rules.enabled(featureNFTokenMintOffer) && - (ctx.tx.isFieldPresent(sfAmount) || - ctx.tx.isFieldPresent(sfDestination) || - ctx.tx.isFieldPresent(sfExpiration))) + bool const hasOfferFields = ctx.tx.isFieldPresent(sfAmount) || + ctx.tx.isFieldPresent(sfDestination) || + ctx.tx.isFieldPresent(sfExpiration); + + if (!ctx.rules.enabled(featureNFTokenMintOffer) && hasOfferFields) return temDISABLED; if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) @@ -86,40 +93,29 @@ NFTokenMint::preflight(PreflightContext const& ctx) return temMALFORMED; } - // Amount field should be set if Destination and/or Expiration is set - if (ctx.tx.isFieldPresent(sfDestination) || - ctx.tx.isFieldPresent(sfExpiration)) + if (hasOfferFields) { + // The Amount field must be present if either the Destination or + // Expiration fields are present. if (!ctx.tx.isFieldPresent(sfAmount)) return temMALFORMED; - } - - if (auto const amount = ctx.tx[~sfAmount]) - { - if (amount->negative() && ctx.rules.enabled(fixNFTokenNegOffer)) - // An offer for a negative amount makes no sense. - return temBAD_AMOUNT; - if (!isXRP(*amount)) + // Rely on the common code shared with NFTokenCreateOffer to + // do the validation. We pass tfSellNFToken as the transaction flags + // because a Mint is only allowed to create a sell offer. + if (NotTEC notTec = nft::tokenOfferCreatePreflight( + ctx.tx[sfAccount], + ctx.tx[sfAmount], + ctx.tx[~sfDestination], + ctx.tx[~sfExpiration], + extractNFTokenFlagsFromTxFlags(ctx.tx.getFlags()), + ctx.rules); + !isTesSuccess(notTec)) { - if (ctx.tx.isFlag(tfOnlyXRP)) - return temBAD_AMOUNT; - - if (!*amount) - return temBAD_AMOUNT; + return notTec; } } - if (auto const exp = ctx.tx[~sfExpiration]; exp == 0) - return temBAD_EXPIRATION; - - if (auto const dest = ctx.tx[~sfDestination]) - { - // The destination can't be the account executing the transaction. - if (dest == ctx.tx[sfAccount]) - return temMALFORMED; - } - return preflight2(ctx); } @@ -186,56 +182,26 @@ NFTokenMint::preclaim(PreclaimContext const& ctx) return tecNO_PERMISSION; } - if (auto const amount = ctx.tx[~sfAmount]) + if (ctx.tx.isFieldPresent(sfAmount)) { + // The Amount field says create an offer for the minted token. if (hasExpired(ctx.view, ctx.tx[~sfExpiration])) return tecEXPIRED; - if (!(ctx.tx.getFlags() & nft::flagCreateTrustLines) && - !amount->native() && ctx.tx[~sfTransferFee]) - { - auto const nftIssuer = - ctx.tx[~sfIssuer].value_or(ctx.tx[sfAccount]); - - // If the IOU issuer and the NFToken issuer are the same, - // then that issuer does not need a trust line to accept their fee. - if (nftIssuer != amount->getIssuer() && - !ctx.view.read(keylet::line(nftIssuer, amount->issue()))) - return tecNO_LINE; - - if (isFrozen( - ctx.view, - nftIssuer, - amount->getCurrency(), - amount->getIssuer())) - return tecFROZEN; - } - - if (isFrozen( + // Rely on the common code shared with NFTokenCreateOffer to + // do the validation. We pass tfSellNFToken as the transaction flags + // because a Mint is only allowed to create a sell offer. + if (TER const ter = nft::tokenOfferCreatePreclaim( ctx.view, ctx.tx[sfAccount], - amount->getCurrency(), - amount->getIssuer())) - return tecFROZEN; - - if (auto const destination = ctx.tx[~sfDestination]) - { - // If a destination is specified, the destination must already be in - // the ledger. - auto const sleDst = ctx.view.read(keylet::account(*destination)); - - if (!sleDst) - return tecNO_DST; - - // check if the destination has disallowed incoming offers - if (ctx.view.rules().enabled(featureDisallowIncoming)) - { - // flag cannot be set unless amendment is enabled but - // out of an abundance of caution check anyway - if (sleDst->getFlags() & lsfDisallowIncomingNFTokenOffer) - return tecNO_PERMISSION; - } - } + ctx.tx[~sfIssuer].value_or(ctx.tx[sfAccount]), + ctx.tx[sfAmount], + ctx.tx[~sfDestination], + extractNFTokenFlagsFromTxFlags(ctx.tx.getFlags()), + ctx.tx[~sfTransferFee].value_or(0), + ctx.j); + !isTesSuccess(ter)) + return ter; } return tesSUCCESS; } @@ -330,7 +296,7 @@ NFTokenMint::doApply() return tecINTERNAL; auto const nftokenID = createNFTokenID( - static_cast(ctx_.tx.getFlags() & 0x0000FFFF), + extractNFTokenFlagsFromTxFlags(ctx_.tx.getFlags()), ctx_.tx[~sfTransferFee].value_or(0), issuer, nft::toTaxon(ctx_.tx[sfNFTokenTaxon]), @@ -351,64 +317,27 @@ NFTokenMint::doApply() if (ctx_.tx.isFieldPresent(sfAmount)) { - auto const offerID = - keylet::nftoffer(account_, ctx_.tx.getSeqProxy().value()); - - // Create the offer: - { - // Token offers are always added to the owner's owner directory: - auto const ownerNode = ctx_.view().dirInsert( - keylet::ownerDir(account_), - offerID, - describeOwnerDir(account_)); - - if (!ownerNode) - return tecDIR_FULL; - - // Token offers are also added to the token's sell offer - // directory - auto const offerNode = ctx_.view().dirInsert( - keylet::nft_sells(nftokenID), - offerID, - [&nftokenID](std::shared_ptr const& sle) { - (*sle)[sfFlags] = lsfNFTokenSellOffers; - (*sle)[sfNFTokenID] = nftokenID; - }); - - if (!offerNode) - return tecDIR_FULL; - - // Only sell offer are supported at NFTokenMint. - std::uint32_t const sleFlags = lsfSellNFToken; - - auto offer = std::make_shared(offerID); - (*offer)[sfOwner] = account_; - (*offer)[sfNFTokenID] = nftokenID; - (*offer)[sfAmount] = ctx_.tx[sfAmount]; - (*offer)[sfFlags] = sleFlags; - (*offer)[sfOwnerNode] = *ownerNode; - (*offer)[sfNFTokenOfferNode] = *offerNode; - - if (auto const expiration = ctx_.tx[~sfExpiration]) - (*offer)[sfExpiration] = *expiration; - - if (auto const destination = ctx_.tx[~sfDestination]) - (*offer)[sfDestination] = *destination; - - ctx_.view().insert(offer); - - adjustOwnerCount( - ctx_.view(), - ctx_.view().peek(keylet::account(account_)), - 1, + // Rely on the common code shared with NFTokenCreateOffer to create + // the offer. We pass tfSellNFToken as the transaction flags + // because a Mint is only allowed to create a sell offer. + if (TER const ter = nft::tokenOfferCreateApply( + view(), + ctx_.tx[sfAccount], + ctx_.tx[sfAmount], + ctx_.tx[~sfDestination], + ctx_.tx[~sfExpiration], + ctx_.tx.getSeqProxy(), + nftokenID, + mPriorBalance, j_); - } + !isTesSuccess(ter)) + return ter; } // Only check the reserve if the owner count actually changed. This // allows NFTs to be added to the page (and burn fees) without // requiring the reserve to be met each time. The reserve is - // only managed when a new NFT page is added. + // only managed when a new NFT page or sell offer is added. if (auto const ownerCountAfter = view().read(keylet::account(account_))->getFieldU32(sfOwnerCount); ownerCountAfter > ownerCountBefore) diff --git a/src/ripple/app/tx/impl/details/NFTokenUtils.cpp b/src/ripple/app/tx/impl/details/NFTokenUtils.cpp index 09ff8f13caa..4acc00f956e 100644 --- a/src/ripple/app/tx/impl/details/NFTokenUtils.cpp +++ b/src/ripple/app/tx/impl/details/NFTokenUtils.cpp @@ -636,5 +636,252 @@ deleteTokenOffer(ApplyView& view, std::shared_ptr const& offer) return true; } +NotTEC +tokenOfferCreatePreflight( + AccountID const& acctID, + STAmount const& amount, + std::optional const& dest, + std::optional const& expiration, + std::uint16_t nftFlags, + Rules const& rules, + std::optional const& owner, + std::uint32_t txFlags) +{ + if (amount.negative() && rules.enabled(fixNFTokenNegOffer)) + // An offer for a negative amount makes no sense. + return temBAD_AMOUNT; + + if (!isXRP(amount)) + { + if (nftFlags & nft::flagOnlyXRP) + return temBAD_AMOUNT; + + if (!amount) + return temBAD_AMOUNT; + } + + // If this is an offer to buy, you must offer something; if it's an + // offer to sell, you can ask for nothing. + bool const isSellOffer = txFlags & tfSellNFToken; + if (!isSellOffer && !amount) + return temBAD_AMOUNT; + + if (expiration.has_value() && expiration.value() == 0) + return temBAD_EXPIRATION; + + // The 'Owner' field must be present when offering to buy, but can't + // be present when selling (it's implicit): + if (owner.has_value() == isSellOffer) + return temMALFORMED; + + if (owner && owner == acctID) + return temMALFORMED; + + if (dest) + { + // Some folks think it makes sense for a buy offer to specify a + // specific broker using the Destination field. This change doesn't + // deserve it's own amendment, so we're piggy-backing on + // fixNFTokenNegOffer. + // + // Prior to fixNFTokenNegOffer any use of the Destination field on + // a buy offer was malformed. + if (!isSellOffer && !rules.enabled(fixNFTokenNegOffer)) + return temMALFORMED; + + // The destination can't be the account executing the transaction. + if (dest == acctID) + return temMALFORMED; + } + return tesSUCCESS; +} + +TER +tokenOfferCreatePreclaim( + ReadView const& view, + AccountID const& acctID, + AccountID const& nftIssuer, + STAmount const& amount, + std::optional const& dest, + std::uint16_t nftFlags, + std::uint16_t xferFee, + beast::Journal j, + std::optional const& owner, + std::uint32_t txFlags) +{ + if (!(nftFlags & nft::flagCreateTrustLines) && !amount.native() && xferFee) + { + if (!view.exists(keylet::account(nftIssuer))) + return tecNO_ISSUER; + + // If the IOU issuer and the NFToken issuer are the same, then that + // issuer does not need a trust line to accept their fee. + if (view.rules().enabled(featureNFTokenMintOffer)) + { + if (nftIssuer != amount.getIssuer() && + !view.read(keylet::line(nftIssuer, amount.issue()))) + return tecNO_LINE; + } + else if (!view.exists(keylet::line(nftIssuer, amount.issue()))) + { + return tecNO_LINE; + } + + if (isFrozen(view, nftIssuer, amount.getCurrency(), amount.getIssuer())) + return tecFROZEN; + } + + if (nftIssuer != acctID && !(nftFlags & nft::flagTransferable)) + { + auto const root = view.read(keylet::account(nftIssuer)); + assert(root); + + if (auto minter = (*root)[~sfNFTokenMinter]; minter != acctID) + return tefNFTOKEN_IS_NOT_TRANSFERABLE; + } + + if (isFrozen(view, acctID, amount.getCurrency(), amount.getIssuer())) + return tecFROZEN; + + // If this is an offer to buy the token, the account must have the + // needed funds at hand; but note that funds aren't reserved and the + // offer may later become unfunded. + if ((txFlags & tfSellNFToken) == 0) + { + // After this amendment, we allow an IOU issuer to make a buy offer + // using their own currency. + if (view.rules().enabled(fixNonFungibleTokensV1_2)) + { + if (accountFunds( + view, acctID, amount, FreezeHandling::fhZERO_IF_FROZEN, j) + .signum() <= 0) + return tecUNFUNDED_OFFER; + } + else if ( + accountHolds( + view, + acctID, + amount.getCurrency(), + amount.getIssuer(), + FreezeHandling::fhZERO_IF_FROZEN, + j) + .signum() <= 0) + return tecUNFUNDED_OFFER; + } + + if (dest) + { + // If a destination is specified, the destination must already be in + // the ledger. + auto const sleDst = view.read(keylet::account(*dest)); + + if (!sleDst) + return tecNO_DST; + + // check if the destination has disallowed incoming offers + if (view.rules().enabled(featureDisallowIncoming)) + { + // flag cannot be set unless amendment is enabled but + // out of an abundance of caution check anyway + + if (sleDst->getFlags() & lsfDisallowIncomingNFTokenOffer) + return tecNO_PERMISSION; + } + } + + if (owner) + { + // Check if the owner (buy offer) has disallowed incoming offers + if (view.rules().enabled(featureDisallowIncoming)) + { + auto const sleOwner = view.read(keylet::account(*owner)); + + // defensively check + // it should not be possible to specify owner that doesn't exist + if (!sleOwner) + return tecNO_TARGET; + + if (sleOwner->getFlags() & lsfDisallowIncomingNFTokenOffer) + return tecNO_PERMISSION; + } + } + + return tesSUCCESS; +} + +TER +tokenOfferCreateApply( + ApplyView& view, + AccountID const& acctID, + STAmount const& amount, + std::optional const& dest, + std::optional const& expiration, + SeqProxy seqProxy, + uint256 const& nftokenID, + XRPAmount const& priorBalance, + beast::Journal j, + std::uint32_t txFlags) +{ + Keylet const acctKeylet = keylet::account(acctID); + if (auto const acct = view.read(acctKeylet); + priorBalance < view.fees().accountReserve((*acct)[sfOwnerCount] + 1)) + return tecINSUFFICIENT_RESERVE; + + auto const offerID = keylet::nftoffer(acctID, seqProxy.value()); + + // Create the offer: + { + // Token offers are always added to the owner's owner directory: + auto const ownerNode = view.dirInsert( + keylet::ownerDir(acctID), offerID, describeOwnerDir(acctID)); + + if (!ownerNode) + return tecDIR_FULL; + + bool const isSellOffer = txFlags & tfSellNFToken; + + // Token offers are also added to the token's buy or sell offer + // directory + auto const offerNode = view.dirInsert( + isSellOffer ? keylet::nft_sells(nftokenID) + : keylet::nft_buys(nftokenID), + offerID, + [&nftokenID, isSellOffer](std::shared_ptr const& sle) { + (*sle)[sfFlags] = + isSellOffer ? lsfNFTokenSellOffers : lsfNFTokenBuyOffers; + (*sle)[sfNFTokenID] = nftokenID; + }); + + if (!offerNode) + return tecDIR_FULL; + + std::uint32_t sleFlags = 0; + + if (isSellOffer) + sleFlags |= lsfSellNFToken; + + auto offer = std::make_shared(offerID); + (*offer)[sfOwner] = acctID; + (*offer)[sfNFTokenID] = nftokenID; + (*offer)[sfAmount] = amount; + (*offer)[sfFlags] = sleFlags; + (*offer)[sfOwnerNode] = *ownerNode; + (*offer)[sfNFTokenOfferNode] = *offerNode; + + if (expiration) + (*offer)[sfExpiration] = *expiration; + + if (dest) + (*offer)[sfDestination] = *dest; + + view.insert(offer); + } + + // Update owner count. + adjustOwnerCount(view, view.peek(acctKeylet), 1, j); + + return tesSUCCESS; +} + } // namespace nft } // namespace ripple diff --git a/src/ripple/app/tx/impl/details/NFTokenUtils.h b/src/ripple/app/tx/impl/details/NFTokenUtils.h index 5242bf38ff3..48ed3993357 100644 --- a/src/ripple/app/tx/impl/details/NFTokenUtils.h +++ b/src/ripple/app/tx/impl/details/NFTokenUtils.h @@ -20,6 +20,7 @@ #ifndef RIPPLE_TX_IMPL_DETAILS_NFTOKENUTILS_H_INCLUDED #define RIPPLE_TX_IMPL_DETAILS_NFTOKENUTILS_H_INCLUDED +#include #include #include #include @@ -97,6 +98,46 @@ deleteTokenOffer(ApplyView& view, std::shared_ptr const& offer); bool compareTokens(uint256 const& a, uint256 const& b); +/** Preflight checks shared by NFTokenCreateOffer and NFTokenMint */ +NotTEC +tokenOfferCreatePreflight( + AccountID const& acctID, + STAmount const& amount, + std::optional const& dest, + std::optional const& expiration, + std::uint16_t nftFlags, + Rules const& rules, + std::optional const& owner = std::nullopt, + std::uint32_t txFlags = lsfSellNFToken); + +/** Preclaim checks shared by NFTokenCreateOffer and NFTokenMint */ +TER +tokenOfferCreatePreclaim( + ReadView const& view, + AccountID const& acctID, + AccountID const& nftIssuer, + STAmount const& amount, + std::optional const& dest, + std::uint16_t nftFlags, + std::uint16_t xferFee, + beast::Journal j, + std::optional const& owner = std::nullopt, + std::uint32_t txFlags = lsfSellNFToken); + +/** doApply implementation shared by NFTokenCreateOffer and NFTokenMint */ +TER +tokenOfferCreateApply( + ApplyView& view, + AccountID const& acctID, + STAmount const& amount, + std::optional const& dest, + std::optional const& expiration, + SeqProxy seqProxy, + uint256 const& nftokenID, + XRPAmount const& priorBalance, + beast::Journal j, + std::uint32_t txFlags = lsfSellNFToken); + } // namespace nft } // namespace ripple diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index f6277a61014..0d98a564b9f 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -6616,9 +6616,11 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite token::expiration(lastClose(env) + 25), ter(temDISABLED)); env.close(); + + return; } - if (features[featureNFTokenMintOffer]) + // The remaining tests assume featureNFTokenMintOffer is enabled. { Env env{*this, features}; Account const alice("alice"); @@ -6838,27 +6840,24 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite // Test sell offers with a destination with and without // fixNFTokenNegOffer. - if (features[featureNFTokenMintOffer]) + for (auto const& tweakedFeatures : + {features - fixNFTokenNegOffer - featureNonFungibleTokensV1_1, + features | fixNFTokenNegOffer}) { - for (auto const& tweakedFeatures : - {features - fixNFTokenNegOffer - featureNonFungibleTokensV1_1, - features | fixNFTokenNegOffer}) - { - Env env{*this, tweakedFeatures}; - Account const alice("alice"); + Env env{*this, tweakedFeatures}; + Account const alice("alice"); - env.fund(XRP(1000000), alice); + env.fund(XRP(1000000), alice); - TER const offerCreateTER = tweakedFeatures[fixNFTokenNegOffer] - ? static_cast(temBAD_AMOUNT) - : static_cast(tesSUCCESS); + TER const offerCreateTER = tweakedFeatures[fixNFTokenNegOffer] + ? static_cast(temBAD_AMOUNT) + : static_cast(tesSUCCESS); - // Make offers with negative amounts for the NFTs - env(token::mint(alice), - token::amount(XRP(-2)), - ter(offerCreateTER)); - env.close(); - } + // Make offers with negative amounts for the NFTs + env(token::mint(alice), + token::amount(XRP(-2)), + ter(offerCreateTER)); + env.close(); } } @@ -7417,6 +7416,164 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite } } + void + testNFTIssuerIsIOUIssuer(FeatureBitset features) + { + testcase("Test fix NFT issuer is IOU issuer"); + + using namespace test::jtx; + + Account const issuer{"issuer"}; + Account const becky{"becky"}; + Account const cheri{"cheri"}; + IOU const isISU(issuer["ISU"]); + + // This test case covers issue... + // https://github.com/XRPLF/rippled/issues/4941 + // + // If an NFToken has a transfer fee then, when an offer is accepted, + // a portion of the sale price goes to the issuer. + // + // It is possible for an issuer to issue both an IOU (for remittances) + // and NFTokens. If the issuer's IOU is used to pay for the transfer + // of one of the issuer's NFTokens, then paying the fee for that + // transfer will fail with a tecNO_LINE. + // + // The problem occurs because the NFT code looks for a trust line to + // pay the transfer fee. However the issuer of an IOU does not need + // a trust line to accept their own issuance and, in fact, is not + // allowed to have a trust line to themselves. + // + // This test looks at a situation where transfer of an NFToken is + // prevented by this bug: + // 1. Issuer issues an IOU (e.g, isISU). + // 2. Becky and Cheri get trust lines for, and acquire, some isISU. + // 3. Issuer mints NFToken with transfer fee. + // 4. Becky acquires the NFToken, paying with XRP. + // 5. Becky attempts to create an offer to sell the NFToken for + // isISU(100). The attempt fails with `tecNO_LINE`. + // + // The featureNFTokenMintOffer amendment addresses this oversight. + // + // We remove the fixRemoveNFTokenAutoTrustLine amendment. Otherwise + // we can't create NFTokens with tfTrustLine enabled. + FeatureBitset const localFeatures = + features - fixRemoveNFTokenAutoTrustLine; + + Env env{*this, localFeatures}; + env.fund(XRP(1000), issuer, becky, cheri); + env.close(); + + // Set trust lines so becky and cheri can use isISU. + env(trust(becky, isISU(1000))); + env(trust(cheri, isISU(1000))); + env.close(); + env(pay(issuer, cheri, isISU(500))); + env.close(); + + // issuer creates two NFTs: one with and one without AutoTrustLine. + std::uint16_t xferFee = 5000; // 5% + uint256 const nftAutoTrustID{token::getNextID( + env, issuer, 0u, tfTransferable | tfTrustLine, xferFee)}; + env(token::mint(issuer, 0u), + token::xferFee(xferFee), + txflags(tfTransferable | tfTrustLine)); + env.close(); + + uint256 const nftNoAutoTrustID{ + token::getNextID(env, issuer, 0u, tfTransferable, xferFee)}; + env(token::mint(issuer, 0u), + token::xferFee(xferFee), + txflags(tfTransferable)); + env.close(); + + // becky buys the nfts for 1 drop each. + { + uint256 const beckyBuyOfferIndex1 = + keylet::nftoffer(becky, env.seq(becky)).key; + env(token::createOffer(becky, nftAutoTrustID, drops(1)), + token::owner(issuer)); + + uint256 const beckyBuyOfferIndex2 = + keylet::nftoffer(becky, env.seq(becky)).key; + env(token::createOffer(becky, nftNoAutoTrustID, drops(1)), + token::owner(issuer)); + + env.close(); + env(token::acceptBuyOffer(issuer, beckyBuyOfferIndex1)); + env(token::acceptBuyOffer(issuer, beckyBuyOfferIndex2)); + env.close(); + } + + // Behavior from here down diverges significantly based on + // featureNFTokenMintOffer. + if (!localFeatures[featureNFTokenMintOffer]) + { + // Without featureNFTokenMintOffer becky simply can't + // create an offer for a non-tfTrustLine NFToken that would + // pay the transfer fee in issuer's own IOU. + env(token::createOffer(becky, nftNoAutoTrustID, isISU(100)), + txflags(tfSellNFToken), + ter(tecNO_LINE)); + env.close(); + + // And issuer can't create a trust line to themselves. + env(trust(issuer, isISU(1000)), ter(temDST_IS_SRC)); + env.close(); + + // However if the NFToken has the tfTrustLine flag set, + // then becky can create the offer. + uint256 const beckyAutoTrustOfferIndex = + keylet::nftoffer(becky, env.seq(becky)).key; + env(token::createOffer(becky, nftAutoTrustID, isISU(100)), + txflags(tfSellNFToken)); + env.close(); + + // And cheri can accept the offer. + env(token::acceptSellOffer(cheri, beckyAutoTrustOfferIndex)); + env.close(); + + // We verify that issuer got their transfer fee by seeing that + // ISU(5) has disappeared out of cheri's and becky's balances. + BEAST_EXPECT(env.balance(becky, isISU) == isISU(95)); + BEAST_EXPECT(env.balance(cheri, isISU) == isISU(400)); + } + else + { + // With featureNFTokenMintOffer things go better. + // becky creates offers to sell the nfts for ISU. + uint256 const beckyNoAutoTrustOfferIndex = + keylet::nftoffer(becky, env.seq(becky)).key; + env(token::createOffer(becky, nftNoAutoTrustID, isISU(100)), + txflags(tfSellNFToken)); + env.close(); + uint256 const beckyAutoTrustOfferIndex = + keylet::nftoffer(becky, env.seq(becky)).key; + env(token::createOffer(becky, nftAutoTrustID, isISU(100)), + txflags(tfSellNFToken)); + env.close(); + + // cheri accepts becky's offers. Behavior is uniform: + // issuer gets paid. + env(token::acceptSellOffer(cheri, beckyAutoTrustOfferIndex)); + env.close(); + + // We verify that issuer got their transfer fee by seeing that + // ISU(5) has disappeared out of cheri's and becky's balances. + BEAST_EXPECT(env.balance(becky, isISU) == isISU(95)); + BEAST_EXPECT(env.balance(cheri, isISU) == isISU(400)); + + env(token::acceptSellOffer(cheri, beckyNoAutoTrustOfferIndex)); + env.close(); + + // We verify that issuer got their transfer fee by seeing that + // an additional ISU(5) has disappeared out of cheri's and + // becky's balances. + BEAST_EXPECT(env.balance(becky, isISU) == isISU(190)); + BEAST_EXPECT(env.balance(cheri, isISU) == isISU(300)); + } + } + void testWithFeats(FeatureBitset features) { @@ -7452,6 +7609,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite testFeatMintWithOffer(features); testTxJsonMetaFields(features); testFixNFTokenBuyerReserve(features); + testNFTIssuerIsIOUIssuer(features); } public: