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 c26fb1fb12a..daffd29ed31 100644 --- a/src/ripple/app/tx/impl/NFTokenMint.cpp +++ b/src/ripple/app/tx/impl/NFTokenMint.cpp @@ -31,12 +31,25 @@ 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; + 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)) return ret; @@ -80,6 +93,29 @@ NFTokenMint::preflight(PreflightContext const& ctx) return temMALFORMED; } + if (hasOfferFields) + { + // The Amount field must be present if either the Destination or + // Expiration fields are present. + if (!ctx.tx.isFieldPresent(sfAmount)) + return temMALFORMED; + + // 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)) + { + return notTec; + } + } + return preflight2(ctx); } @@ -146,6 +182,27 @@ NFTokenMint::preclaim(PreclaimContext const& ctx) return tecNO_PERMISSION; } + 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; + + // 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], + 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; } @@ -238,18 +295,16 @@ NFTokenMint::doApply() // Should never happen. return tecINTERNAL; + auto const nftokenID = createNFTokenID( + extractNFTokenFlagsFromTxFlags(ctx_.tx.getFlags()), + ctx_.tx[~sfTransferFee].value_or(0), + issuer, + nft::toTaxon(ctx_.tx[sfNFTokenTaxon]), + tokenSeq.value()); + STObject newToken( - *nfTokenTemplate, - sfNFToken, - [this, &issuer, &tokenSeq](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())); + *nfTokenTemplate, sfNFToken, [this, &nftokenID](STObject& object) { + object.setFieldH256(sfNFTokenID, nftokenID); if (auto const uri = ctx_.tx[~sfURI]) object.setFieldVL(sfURI, *uri); @@ -260,10 +315,29 @@ NFTokenMint::doApply() ret != tesSUCCESS) return ret; + if (ctx_.tx.isFieldPresent(sfAmount)) + { + // 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/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index 78863908c25..2119a954ca9 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -80,7 +80,7 @@ namespace detail { // Feature.cpp. Because it's only used to reserve storage, and determine how // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than // the actual number of amendments. A LogicError on startup will verify this. -static constexpr std::size_t numFeatures = 74; +static constexpr std::size_t numFeatures = 75; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated @@ -366,6 +366,7 @@ extern uint256 const fixEmptyDID; extern uint256 const fixXChainRewardRounding; extern uint256 const fixPreviousTxnID; extern uint256 const fixAMMv1_1; +extern uint256 const featureNFTokenMintOffer; extern uint256 const fixReducedOffersV2; } // namespace ripple diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 6543498cef1..a1b73dbea8b 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -493,6 +493,7 @@ REGISTER_FIX (fixEmptyDID, Supported::yes, VoteBehavior::De REGISTER_FIX (fixXChainRewardRounding, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FIX (fixPreviousTxnID, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FIX (fixAMMv1_1, Supported::yes, VoteBehavior::DefaultNo); +REGISTER_FEATURE(NFTokenMintOffer, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FIX (fixReducedOffersV2, Supported::yes, VoteBehavior::DefaultNo); // The following amendments are obsolete, but must remain supported 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/ripple/protocol/impl/TxFormats.cpp b/src/ripple/protocol/impl/TxFormats.cpp index d2bdd4f8aa7..324c0e30790 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 9a5ace19b1e..c8531273917 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -3171,6 +3171,26 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite ter(tecNO_PERMISSION)); env.close(); } + + // 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(); + + // disable flag + env(fclear(buyer, asfDisallowIncomingNFTokenOffer)); + env(token::mint(minter), + token::amount(drops(1)), + token::destination(buyer)); + env.close(); + } } void @@ -6566,6 +6586,281 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite } } + void + testFeatMintWithOffer(FeatureBitset features) + { + testcase("NFTokenMint with Create NFTokenOffer"); + + using namespace test::jtx; + + if (!features[featureNFTokenMintOffer]) + { + Env env{*this, features}; + Account const alice("alice"); + Account const buyer("buyer"); + + 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(); + + return; + } + + // The remaining tests assume featureNFTokenMintOffer is enabled. + { + Env env{*this, features}; + Account const alice("alice"); + Account const buyer{"buyer"}; + 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); + 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); + + // 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 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); + } + + { + // 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); + + // 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); + + // 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(); + + // 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(); + } + + { + // 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))); + 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(); + + // 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)), + 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(); + + 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. + 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(); + } + } + void testTxJsonMetaFields(FeatureBitset features) { @@ -6796,6 +7091,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 @@ -7112,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) { @@ -7144,8 +7606,10 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite testIOUWithTransferFee(features); testBrokeredSaleToSelf(features); testFixNFTokenRemint(features); + testFeatMintWithOffer(features); testTxJsonMetaFields(features); testFixNFTokenBuyerReserve(features); + testNFTIssuerIsIOUIssuer(features); } public: @@ -7156,15 +7620,17 @@ class NFTokenBaseUtil_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 - - fixNFTokenReserve, + fixNFTokenReserve - featureNFTokenMintOffer, all - disallowIncoming - fixNonFungibleTokensV1_2 - - fixNFTokenRemint - fixNFTokenReserve, + fixNFTokenRemint - fixNFTokenReserve - featureNFTokenMintOffer, all - fixNonFungibleTokensV1_2 - fixNFTokenRemint - - fixNFTokenReserve, - all - fixNFTokenRemint - fixNFTokenReserve, - all - fixNFTokenReserve, + fixNFTokenReserve - featureNFTokenMintOffer, + all - fixNFTokenRemint - fixNFTokenReserve - + featureNFTokenMintOffer, + all - fixNFTokenReserve - featureNFTokenMintOffer, + all - featureNFTokenMintOffer, all}; if (BEAST_EXPECT(instance < feats.size())) @@ -7217,12 +7683,21 @@ class NFTokenWOTokenReserve_test : public NFTokenBaseUtil_test } }; +class NFTokenWOMintOffer_test : public NFTokenBaseUtil_test +{ + void + run() override + { + NFTokenBaseUtil_test::run(5); + } +}; + class NFTokenAllFeatures_test : public NFTokenBaseUtil_test { void run() override { - NFTokenBaseUtil_test::run(5, true); + NFTokenBaseUtil_test::run(6, true); } }; @@ -7231,6 +7706,7 @@ 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(NFTokenWOTokenReserve, tx, ripple, 2); +BEAST_DEFINE_TESTSUITE_PRIO(NFTokenWOMintOffer, tx, ripple, 2); BEAST_DEFINE_TESTSUITE_PRIO(NFTokenAllFeatures, tx, ripple, 2); } // namespace ripple 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(