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.

Sign up for GitHub

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

fixNFTokenBrokerAccept #8

Closed
wants to merge 1 commit into from
+108 −47
Closed

fixNFTokenBrokerAccept #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 30 additions & 6 deletions src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,20 +112,44 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
if ((*so)[sfAmount] > (*bo)[sfAmount])
return tecINSUFFICIENT_PAYMENT;

// If the buyer specified a destination, that destination must be
// the seller or the broker.
// If the buyer specified a destination
if (auto const dest = bo->at(~sfDestination))
{
if (*dest != so->at(sfOwner) && *dest != ctx.tx[sfAccount])
// fixUnburnableNFToken: Disabled
// the destination must be the seller or the broker.
if (!ctx.view.rules().enabled(fixUnburnableNFToken) &&
*dest != so->at(sfOwner) && *dest != ctx.tx[sfAccount])
{
return tecNFTOKEN_BUY_SELL_MISMATCH;
}

// fixUnburnableNFToken: Enabled
// the destination may only be the account brokering the offer
if (ctx.view.rules().enabled(fixUnburnableNFToken) &&
*dest != ctx.tx[sfAccount])
{
return tecNO_PERMISSION;
}
}

// If the seller specified a destination, that destination must be
// the buyer or the broker.
// If the seller specified a destination
if (auto const dest = so->at(~sfDestination))
{
if (*dest != bo->at(sfOwner) && *dest != ctx.tx[sfAccount])
// fixUnburnableNFToken: Disabled
// the destination must be the buyer or the broker.
if (!ctx.view.rules().enabled(fixUnburnableNFToken) &&
*dest != bo->at(sfOwner) && *dest != ctx.tx[sfAccount])
{
return tecNFTOKEN_BUY_SELL_MISMATCH;
}

// fixUnburnableNFToken: Enabled
// the destination may only be the account brokering the offer
if (ctx.view.rules().enabled(fixUnburnableNFToken) &&
*dest != ctx.tx[sfAccount])
{
return tecNO_PERMISSION;
}
}

// The broker can specify an amount that represents their cut; if they
Expand Down
3 changes: 2 additions & 1 deletion src/ripple/protocol/Feature.h
View file
Open in desktop
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 = 54;
static constexpr std::size_t numFeatures = 55;

/** 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 @@ -341,6 +341,7 @@ extern uint256 const fixTrustLinesToSelf;
extern uint256 const fixRemoveNFTokenAutoTrustLine;
extern uint256 const featureImmediateOfferKilled;
extern uint256 const featureDisallowIncoming;
extern uint256 const fixUnburnableNFToken;

} // namespace ripple

Expand Down
1 change: 1 addition & 0 deletions src/ripple/protocol/impl/Feature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +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);

// 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
115 changes: 75 additions & 40 deletions src/test/app/NFToken_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2872,15 +2872,21 @@ class NFToken_test : public beast::unit_test::suite
BEAST_EXPECT(ownerCount(env, minter) == 2);
BEAST_EXPECT(ownerCount(env, buyer) == 1);

// issuer cannot broker the offers, because they are not the
// Destination.
env(token::brokerOffers(
issuer, offerBuyerToMinter, offerMinterToBroker),
ter(tecNFTOKEN_BUY_SELL_MISMATCH));
env.close();
BEAST_EXPECT(ownerCount(env, issuer) == 0);
BEAST_EXPECT(ownerCount(env, minter) == 2);
BEAST_EXPECT(ownerCount(env, buyer) == 1);
// fixUnburnableNFToken: Disabled
{
// issuer cannot broker the offers, because they are not the
// Destination.
TER const expectTer = features[fixUnburnableNFToken]
? tecNO_PERMISSION
: tecNFTOKEN_BUY_SELL_MISMATCH;
env(token::brokerOffers(
issuer, offerBuyerToMinter, offerMinterToBroker),
ter(expectTer));
env.close();
BEAST_EXPECT(ownerCount(env, issuer) == 0);
BEAST_EXPECT(ownerCount(env, minter) == 2);
BEAST_EXPECT(ownerCount(env, buyer) == 1);
}

// Since broker is the sell offer's destination, they can broker
// the two offers.
Expand Down Expand Up @@ -2917,29 +2923,51 @@ class NFToken_test : public beast::unit_test::suite
BEAST_EXPECT(ownerCount(env, minter) == 1);
BEAST_EXPECT(ownerCount(env, buyer) == 2);

// Cannot broker offers when the sell destination is not the buyer.
env(token::brokerOffers(
broker, offerIssuerToBuyer, offerBuyerToMinter),
ter(tecNFTOKEN_BUY_SELL_MISMATCH));
env.close();
BEAST_EXPECT(ownerCount(env, issuer) == 1);
BEAST_EXPECT(ownerCount(env, minter) == 1);
BEAST_EXPECT(ownerCount(env, buyer) == 2);
{
// Cannot broker offers when the sell destination is not the
// buyer.
TER const expectTer = features[fixUnburnableNFToken]
? tecNO_PERMISSION
: tecNFTOKEN_BUY_SELL_MISMATCH;
env(token::brokerOffers(
broker, offerIssuerToBuyer, offerBuyerToMinter),
ter(expectTer));
env.close();

// Broker is successful when destination is buyer.
env(token::brokerOffers(
broker, offerMinterToBuyer, offerBuyerToMinter));
env.close();
BEAST_EXPECT(ownerCount(env, issuer) == 1);
BEAST_EXPECT(ownerCount(env, minter) == 1);
BEAST_EXPECT(ownerCount(env, buyer) == 0);
BEAST_EXPECT(ownerCount(env, issuer) == 1);
BEAST_EXPECT(ownerCount(env, minter) == 1);
BEAST_EXPECT(ownerCount(env, buyer) == 2);

// Clean out the unconsumed offer.
env(token::cancelOffer(issuer, {offerIssuerToBuyer}));
env.close();
BEAST_EXPECT(ownerCount(env, issuer) == 0);
BEAST_EXPECT(ownerCount(env, minter) == 1);
BEAST_EXPECT(ownerCount(env, buyer) == 0);
// Broker is successful when destination is buyer.
TER const eexpectTer = features[fixUnburnableNFToken]
? tecNO_PERMISSION
: TER(tesSUCCESS);
env(token::brokerOffers(
broker, offerMinterToBuyer, offerBuyerToMinter),
ter(eexpectTer));
env.close();

if (features[fixUnburnableNFToken])
// Buyer is successful with acceptOffer.
env(token::acceptBuyOffer(buyer, offerMinterToBuyer));
env.close();

// Clean out the unconsumed offer.
env(token::cancelOffer(buyer, {offerBuyerToMinter}));
env.close();

BEAST_EXPECT(ownerCount(env, issuer) == 1);
BEAST_EXPECT(ownerCount(env, minter) == 1);
BEAST_EXPECT(ownerCount(env, buyer) == 0);

// Clean out the unconsumed offer.
env(token::cancelOffer(issuer, {offerIssuerToBuyer}));
env.close();
BEAST_EXPECT(ownerCount(env, issuer) == 0);
BEAST_EXPECT(ownerCount(env, minter) == 1);
BEAST_EXPECT(ownerCount(env, buyer) == 0);
return;
}
}

// Show that if a buy and a sell offer both have the same destination,
Expand All @@ -2957,15 +2985,20 @@ class NFToken_test : public beast::unit_test::suite
token::owner(minter),
token::destination(broker));

// Cannot broker offers when the sell destination is not the buyer
// or the broker.
env(token::brokerOffers(
issuer, offerBuyerToBroker, offerMinterToBroker),
ter(tecNFTOKEN_BUY_SELL_MISMATCH));
env.close();
BEAST_EXPECT(ownerCount(env, issuer) == 0);
BEAST_EXPECT(ownerCount(env, minter) == 2);
BEAST_EXPECT(ownerCount(env, buyer) == 1);
{
// Cannot broker offers when the sell destination is not the
// buyer or the broker.
TER const expectTer = features[fixUnburnableNFToken]
? tecNO_PERMISSION
: tecNFTOKEN_BUY_SELL_MISMATCH;
env(token::brokerOffers(
issuer, offerBuyerToBroker, offerMinterToBroker),
ter(expectTer));
env.close();
BEAST_EXPECT(ownerCount(env, issuer) == 0);
BEAST_EXPECT(ownerCount(env, minter) == 2);
BEAST_EXPECT(ownerCount(env, buyer) == 1);
}

// Broker is successful if they are the destination of both offers.
env(token::brokerOffers(
Expand Down Expand Up @@ -5079,13 +5112,15 @@ class NFToken_test : public beast::unit_test::suite
using namespace test::jtx;
FeatureBitset const all{supported_amendments()};
FeatureBitset const fixNFTDir{fixNFTokenDirV1};
FeatureBitset const fixNFTBroker{fixUnburnableNFToken};

testWithFeats(all - fixNFTDir);
testWithFeats(all - disallowIncoming);
testWithFeats(all - fixNFTBroker);
testWithFeats(all);
}
};

BEAST_DEFINE_TESTSUITE_PRIO(NFToken, tx, ripple, 2);

} // namespace ripple
} // namespace ripple