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

rename fixUnburnableNFToken to fixNonFungibleTokensV1_2 and some cosmetic changes #4419

Merged
merged 1 commit into from
Feb 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
36 changes: 14 additions & 22 deletions src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)

// The two offers may not form a loop. A broker may not sell the
// token to the current owner of the token.
if (ctx.view.rules().enabled(fixUnburnableNFToken) &&
if (ctx.view.rules().enabled(fixNonFungibleTokensV1_2) &&
((*bo)[sfOwner] == (*so)[sfOwner]))
return tecCANT_ACCEPT_OWN_NFTOKEN_OFFER;

Expand All @@ -121,37 +121,29 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
// If the buyer specified a destination
if (auto const dest = bo->at(~sfDestination))
{
// fixUnburnableNFToken
if (ctx.view.rules().enabled(fixUnburnableNFToken))
// Before this fix the destination could be either the seller or
// a broker. After, it must be whoever is submitting the tx.
if (ctx.view.rules().enabled(fixNonFungibleTokensV1_2))
{
// the destination may only be the account brokering the offer
if (*dest != ctx.tx[sfAccount])
return tecNO_PERMISSION;
}
else
{
// the destination must be the seller or the broker.
if (*dest != so->at(sfOwner) && *dest != ctx.tx[sfAccount])
return tecNFTOKEN_BUY_SELL_MISMATCH;
}
else if (*dest != so->at(sfOwner) && *dest != ctx.tx[sfAccount])
return tecNFTOKEN_BUY_SELL_MISMATCH;
}

// If the seller specified a destination
if (auto const dest = so->at(~sfDestination))
{
// fixUnburnableNFToken
if (ctx.view.rules().enabled(fixUnburnableNFToken))
// Before this fix the destination could be either the seller or
// a broker. After, it must be whoever is submitting the tx.
if (ctx.view.rules().enabled(fixNonFungibleTokensV1_2))
{
// the destination may only be the account brokering the offer
if (*dest != ctx.tx[sfAccount])
return tecNO_PERMISSION;
}
else
{
// the destination must be the buyer or the broker.
if (*dest != bo->at(sfOwner) && *dest != ctx.tx[sfAccount])
return tecNFTOKEN_BUY_SELL_MISMATCH;
}
else if (*dest != bo->at(sfOwner) && *dest != ctx.tx[sfAccount])
return tecNFTOKEN_BUY_SELL_MISMATCH;
}

// The broker can specify an amount that represents their cut; if they
Expand Down Expand Up @@ -200,7 +192,7 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
// After this amendment, we allow an IOU issuer to buy an NFT with their
// own currency
auto const needed = bo->at(sfAmount);
if (ctx.view.rules().enabled(fixUnburnableNFToken))
if (ctx.view.rules().enabled(fixNonFungibleTokensV1_2))
{
if (accountFunds(
ctx.view, (*bo)[sfOwner], needed, fhZERO_IF_FROZEN, ctx.j) <
Expand Down Expand Up @@ -243,7 +235,7 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)

// The account offering to buy must have funds:
auto const needed = so->at(sfAmount);
if (!ctx.view.rules().enabled(fixUnburnableNFToken))
if (!ctx.view.rules().enabled(fixNonFungibleTokensV1_2))
{
if (accountHolds(
ctx.view,
Expand Down Expand Up @@ -298,7 +290,7 @@ NFTokenAcceptOffer::pay(
// their own currency, we know that something went wrong. This was
// originally found in the context of IOU transfer fees. Since there are
// several payouts in this tx, just confirm that the end state is OK.
if (!view().rules().enabled(fixUnburnableNFToken))
if (!view().rules().enabled(fixNonFungibleTokensV1_2))
return result;
if (result != tesSUCCESS)
return result;
Expand Down
4 changes: 2 additions & 2 deletions src/ripple/app/tx/impl/NFTokenBurn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ NFTokenBurn::preclaim(PreclaimContext const& ctx)
}
}

if (!ctx.view.rules().enabled(fixUnburnableNFToken))
if (!ctx.view.rules().enabled(fixNonFungibleTokensV1_2))
{
// If there are too many offers, then burning the token would produce
// too much metadata. Disallow burning a token with too many offers.
Expand Down Expand Up @@ -109,7 +109,7 @@ NFTokenBurn::doApply()
view().update(issuer);
}

if (ctx_.view().rules().enabled(fixUnburnableNFToken))
if (ctx_.view().rules().enabled(fixNonFungibleTokensV1_2))
{
// Delete up to 500 offers in total.
// Because the number of sell offers is likely to be less than
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/app/tx/impl/NFTokenCreateOffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ NFTokenCreateOffer::preclaim(PreclaimContext const& ctx)
{
// After this amendment, we allow an IOU issuer to make a buy offer
// using their own currency.
if (ctx.view.rules().enabled(fixUnburnableNFToken))
if (ctx.view.rules().enabled(fixNonFungibleTokensV1_2))
{
if (accountFunds(
ctx.view,
Expand Down
2 changes: 1 addition & 1 deletion src/ripple/protocol/Feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ extern uint256 const fixTrustLinesToSelf;
extern uint256 const fixRemoveNFTokenAutoTrustLine;
extern uint256 const featureImmediateOfferKilled;
extern uint256 const featureDisallowIncoming;
extern uint256 const fixUnburnableNFToken;
extern uint256 const fixNonFungibleTokensV1_2;

} // namespace ripple

Expand Down
2 changes: 1 addition & 1 deletion src/ripple/protocol/impl/Feature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ REGISTER_FIX (fixTrustLinesToSelf, Supported::yes, DefaultVote::no)
REGISTER_FIX (fixRemoveNFTokenAutoTrustLine, Supported::yes, DefaultVote::yes);
REGISTER_FEATURE(ImmediateOfferKilled, Supported::yes, DefaultVote::no);
REGISTER_FEATURE(DisallowIncoming, Supported::yes, DefaultVote::no);
REGISTER_FIX (fixUnburnableNFToken, Supported::yes, DefaultVote::no);
REGISTER_FIX (fixNonFungibleTokensV1_2, Supported::yes, DefaultVote::no);

// The following amendments have been active for at least two years. Their
// pre-amendment code has been removed and the identifiers are deprecated.
Expand Down
24 changes: 12 additions & 12 deletions src/test/app/NFTokenBurn_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -524,8 +524,8 @@ class NFTokenBurn_test : public beast::unit_test::suite
using namespace test::jtx;

// Test what happens if a NFT is unburnable when there are
// more than 500 offers, before fixUnburnableNFToken goes live
if (!features[fixUnburnableNFToken])
// more than 500 offers, before fixNonFungibleTokensV1_2 goes live
if (!features[fixNonFungibleTokensV1_2])
{
Env env{*this, features};

Expand Down Expand Up @@ -620,10 +620,10 @@ class NFTokenBurn_test : public beast::unit_test::suite
}

// Test that up to 499 buy/sell offers will be removed when NFT is
// burned after fixUnburnableNFToken is enabled. This is to test that we
// can successfully remove all offers if the number of offers is less
// than 500.
if (features[fixUnburnableNFToken])
// burned after fixNonFungibleTokensV1_2 is enabled. This is to test
// that we can successfully remove all offers if the number of offers is
// less than 500.
if (features[fixNonFungibleTokensV1_2])
{
Env env{*this, features};

Expand Down Expand Up @@ -673,8 +673,8 @@ class NFTokenBurn_test : public beast::unit_test::suite
}

// Test that up to 500 buy offers are removed when NFT is burned
// after fixUnburnableNFToken is enabled
if (features[fixUnburnableNFToken])
// after fixNonFungibleTokensV1_2 is enabled
if (features[fixNonFungibleTokensV1_2])
{
Env env{*this, features};

Expand Down Expand Up @@ -718,8 +718,8 @@ class NFTokenBurn_test : public beast::unit_test::suite
}

// Test that up to 500 buy/sell offers are removed when NFT is burned
// after fixUnburnableNFToken is enabled
if (features[fixUnburnableNFToken])
// after fixNonFungibleTokensV1_2 is enabled
if (features[fixNonFungibleTokensV1_2])
{
Env env{*this, features};

Expand Down Expand Up @@ -786,8 +786,8 @@ class NFTokenBurn_test : public beast::unit_test::suite
FeatureBitset const all{supported_amendments()};
FeatureBitset const fixNFTDir{fixNFTokenDirV1};

testWithFeats(all - fixUnburnableNFToken - fixNFTDir);
testWithFeats(all - fixUnburnableNFToken);
testWithFeats(all - fixNonFungibleTokensV1_2 - fixNFTDir);
testWithFeats(all - fixNonFungibleTokensV1_2);
testWithFeats(all);
}
};
Expand Down
Loading