Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixNFTokenReserve: Throw error when NFT buyer does not meet reserve requirement #4767

Merged
merged 28 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
68f354d
Fix reserve
shawnxie999 Oct 12, 2023
1f78273
comments
shawnxie999 Oct 17, 2023
45bdc48
Merge branch 'develop' into nft-reserve
shawnxie999 Oct 17, 2023
4a5a2fd
new line
shawnxie999 Oct 17, 2023
f38d04f
Merge branch 'develop' into nft-reserve
shawnxie999 Oct 17, 2023
8e9a6e5
Merge remote-tracking branch 'upstream/develop' into nft-reserve
shawnxie999 Oct 18, 2023
459954e
Merge branch 'develop' into nft-reserve
shawnxie999 Oct 23, 2023
54e857e
Merge branch 'develop' into nft-reserve
shawnxie999 Oct 25, 2023
5bd3781
Address comments
shawnxie999 Oct 30, 2023
809b001
clang
shawnxie999 Oct 30, 2023
c41e18d
Merge branch 'develop' into nft-reserve
shawnxie999 Oct 30, 2023
46c2d66
Merge remote-tracking branch 'upstream/develop' into nft-reserve
shawnxie999 Oct 31, 2023
c037d7f
Revert "Merge remote-tracking branch 'upstream/develop' into nft-rese…
shawnxie999 Oct 31, 2023
8eb9fe6
Merge remote-tracking branch 'upstream/develop' into nft-reserve
shawnxie999 Oct 31, 2023
726c440
act to acct
shawnxie999 Oct 31, 2023
4fdd168
lint
shawnxie999 Oct 31, 2023
553d76b
Merge branch 'develop' into nft-reserve
shawnxie999 Nov 3, 2023
8e3f9d6
Merge branch 'develop' into nft-reserve
shawnxie999 Nov 9, 2023
b661708
Merge branch 'develop' into nft-reserve
shawnxie999 Nov 29, 2023
2f5f09f
Merge branch 'develop' into nft-reserve
shawnxie999 Nov 30, 2023
3411090
Merge branch 'develop' into nft-reserve
shawnxie999 Dec 1, 2023
b7f468f
Merge branch 'develop' into nft-reserve
shawnxie999 Jan 9, 2024
34c3c58
Merge remote-tracking branch 'upstream/develop' into nft-reserve
shawnxie999 Jan 22, 2024
e1a955c
Merge branch 'develop' into nft-reserve
intelliot Jan 22, 2024
d5a6cff
Merge branch 'develop' into nft-reserve
shawnxie999 Jan 24, 2024
74b6c08
Merge branch 'develop' into nft-reserve
shawnxie999 Jan 26, 2024
3329fec
Merge branch 'develop' into nft-reserve
shawnxie999 Jan 30, 2024
cdbbb0d
Merge branch 'develop' into nft-reserve
shawnxie999 Feb 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 57 additions & 22 deletions src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,60 @@ NFTokenAcceptOffer::pay(
return tesSUCCESS;
}

TER
NFTokenAcceptOffer::transferNFToken(
AccountID const& buyer,
AccountID const& seller,
uint256 const& nftokenID)
{
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))
{
// 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.
shawnxie999 marked this conversation as resolved.
Show resolved Hide resolved
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 insertRet;
}

TER
NFTokenAcceptOffer::acceptOffer(std::shared_ptr<SLE> const& offer)
{
Expand Down Expand Up @@ -333,17 +387,7 @@ NFTokenAcceptOffer::acceptOffer(std::shared_ptr<SLE> 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;

return nft::insertToken(view(), buyer, std::move(tokenAndPage->token));
return transferNFToken(buyer, seller, nftokenID);
}

TER
Expand Down Expand Up @@ -431,17 +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;

return nft::insertToken(view(), buyer, std::move(tokenAndPage->token));
// Now transfer the NFT:
return transferNFToken(buyer, seller, nftokenID);
}

if (bo)
Expand Down
6 changes: 6 additions & 0 deletions src/ripple/app/tx/impl/NFTokenAcceptOffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ class NFTokenAcceptOffer : public Transactor
std::shared_ptr<SLE> const& buy,
std::shared_ptr<SLE> const& sell);

TER
transferNFToken(
AccountID const& buyer,
AccountID const& seller,
uint256 const& nfTokenID);

public:
static constexpr ConsequencesFactoryType ConsequencesFactory{Normal};

Expand Down
3 changes: 2 additions & 1 deletion src/ripple/protocol/Feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -352,6 +352,7 @@ extern uint256 const featureXChainBridge;
extern uint256 const fixDisallowIncomingV1;
extern uint256 const featureDID;
extern uint256 const fixFillOrKill;
extern uint256 const fixNFTokenReserve;

} // namespace ripple

Expand Down
1 change: 1 addition & 0 deletions src/ripple/protocol/impl/Feature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,7 @@ REGISTER_FEATURE(XChainBridge, Supported::yes, VoteBehavior::De
REGISTER_FIX (fixDisallowIncomingV1, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FEATURE(DID, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FIX(fixFillOrKill, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FIX (fixNFTokenReserve, Supported::yes, VoteBehavior::DefaultNo);

// The following amendments are obsolete, but must remain supported
// because they could potentially get enabled.
Expand Down
Loading