diff --git a/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp b/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp index 4867eee03a3..02471c1d482 100644 --- a/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp +++ b/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp @@ -302,27 +302,57 @@ NFTokenAcceptOffer::pay( } TER -NFTokenAcceptOffer::checkBuyerReserve( - std::shared_ptr const& sleBuyer, - std::uint32_t const buyerOwnerCountBefore) +NFTokenAcceptOffer::transferNFToken( + AccountID const& buyer, + AccountID const& seller, + uint256 const& nftokenID) { - // To check if there is sufficient reserve, we cannot use mPriorBalance - // because NFT is sold for a price. So we must use the balance after the - // deduction of the potential offer price. A small caveat here is that the - // balance has already deducted the transaction fee, meaning that the - // reserve requirement is a few drops higher. - auto const buyerBalance = sleBuyer->getFieldAmount(sfBalance); - - auto const buyerOwnerCountAfter = sleBuyer->getFieldU32(sfOwnerCount); - if (buyerOwnerCountAfter > buyerOwnerCountBefore) + auto tokenAndPage = nft::findTokenAndPage(view(), seller, nftokenID); + + if (!tokenAndPage) + return tecINTERNAL; + + if (auto const ret = nft::removeToken( + view(), seller, nftokenID, std::move(tokenAndPage->page)); + !isTesSuccess(ret)) + return ret; + + auto const sleBuyer = view().read(keylet::account(buyer)); + if (!sleBuyer) + return tecINTERNAL; + + std::uint32_t const buyerOwnerCountBefore = + sleBuyer->getFieldU32(sfOwnerCount); + + auto const insertRet = + nft::insertToken(view(), buyer, std::move(tokenAndPage->token)); + + // if fixNFTokenReserve is enabled, check if the buyer has sufficient + // reserve to own a new object, if their OwnerCount changed. + // + // There was an issue where the buyer accepts a sell offer, the ledger + // didn't check if the buyer has enough reserve, meaning that buyer can get + // NFTs free of reserve. + if (view().rules().enabled(fixNFTokenReserve)) { - if (auto const reserve = - view().fees().accountReserve(buyerOwnerCountAfter); - buyerBalance < reserve) - return tecINSUFFICIENT_RESERVE; + // To check if there is sufficient reserve, we cannot use mPriorBalance + // because NFT is sold for a price. So we must use the balance after + // the deduction of the potential offer price. A small caveat here is + // that the balance has already deducted the transaction fee, meaning + // that the reserve requirement is a few drops higher. + auto const buyerBalance = sleBuyer->getFieldAmount(sfBalance); + + auto const buyerOwnerCountAfter = sleBuyer->getFieldU32(sfOwnerCount); + if (buyerOwnerCountAfter > buyerOwnerCountBefore) + { + if (auto const reserve = + view().fees().accountReserve(buyerOwnerCountAfter); + buyerBalance < reserve) + return tecINSUFFICIENT_RESERVE; + } } - return tesSUCCESS; + return insertRet; } TER @@ -357,44 +387,7 @@ NFTokenAcceptOffer::acceptOffer(std::shared_ptr const& offer) } // Now transfer the NFT: - auto tokenAndPage = nft::findTokenAndPage(view(), seller, nftokenID); - - if (!tokenAndPage) - return tecINTERNAL; - - if (auto const ret = nft::removeToken( - view(), seller, nftokenID, std::move(tokenAndPage->page)); - !isTesSuccess(ret)) - return ret; - - auto const sleBuyer = view().read(keylet::account(buyer)); - std::uint32_t const buyerOwnerCountBefore = - sleBuyer->getFieldU32(sfOwnerCount); - - auto const insertRet = - nft::insertToken(view(), buyer, std::move(tokenAndPage->token)); - - // if fixNFTokenReserve is enabled, check if the buyer has sufficient - // reserve to own a new object, if their OwnerCount changed. - // - // There was an issue where the buyer accepts a sell offer, the ledger - // didn't check if the buyer has enough reserve, meaning that buyer can get - // NFTs free of reserve. - // - // But, this isn't a problem when a buy offer is involved, because a buy - // offer is already taking up reserve. Since the buy offer is deleted - // near the end of the transaction, its reserve will be freed up, and at the - // same time, the new NFT that the buyer bought could take up that reserve - // again. So it's guareenteed that there is enough reserve as long the - // transactor pass the preclaim. - if (view().rules().enabled(fixNFTokenReserve)) - { - if (auto const ret = checkBuyerReserve(sleBuyer, buyerOwnerCountBefore); - !isTesSuccess(ret)) - return ret; - } - - return insertRet; + return transferNFToken(buyer, seller, nftokenID); } TER @@ -482,38 +475,8 @@ NFTokenAcceptOffer::doApply() return r; } - auto tokenAndPage = nft::findTokenAndPage(view(), seller, nftokenID); - - if (!tokenAndPage) - return tecINTERNAL; - - if (auto const ret = nft::removeToken( - view(), seller, nftokenID, std::move(tokenAndPage->page)); - !isTesSuccess(ret)) - return ret; - - auto const sleBuyer = view().read(keylet::account(buyer)); - std::uint32_t const buyerOwnerCountBefore = - sleBuyer->getFieldU32(sfOwnerCount); - - auto const insertRet = - nft::insertToken(view(), buyer, std::move(tokenAndPage->token)); - - // if fixNFTokenReserve is enabled, check if the buyer has sufficient - // reserve to own a new object, if their OwnerCount changed. - // - // However, this check is merely for safety and should never trigger, - // because, in brokered mode, the buy offer's reserve will be freed up, - // which can be taken up by the new NFT again. So buyer is always - // guarenteed to have enough reserve. - if (view().rules().enabled(fixNFTokenReserve)) - { - if (auto const ret = - checkBuyerReserve(sleBuyer, buyerOwnerCountBefore); - !isTesSuccess(ret)) - return ret; - } - return insertRet; + // Now transfer the NFT: + return transferNFToken(buyer, seller, nftokenID); } if (bo) diff --git a/src/ripple/app/tx/impl/NFTokenAcceptOffer.h b/src/ripple/app/tx/impl/NFTokenAcceptOffer.h index 752f4a768c1..e1b26cbecea 100644 --- a/src/ripple/app/tx/impl/NFTokenAcceptOffer.h +++ b/src/ripple/app/tx/impl/NFTokenAcceptOffer.h @@ -39,9 +39,10 @@ class NFTokenAcceptOffer : public Transactor std::shared_ptr const& sell); TER - checkBuyerReserve( - std::shared_ptr const& sleBuyer, - std::uint32_t const buyerOwnerCountBefore); + transferNFToken( + AccountID const& buyer, + AccountID const& seller, + uint256 const& nfTokenID); public: static constexpr ConsequencesFactoryType ConsequencesFactory{Normal};