ARCHIVE/xahaud
3
0
Fork 0
mirror of https://github.com/Xahau/xahaud.git synced 2025-12-06 17:27:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

Only account specified as destination can settle through brokerage: (#4399)

Browse Source 
Without this amendment, for NFTs using broker mode, if the sell offer contains a destination and that destination is the buyer account, anyone can broker the transaction. Also, if a buy offer contains a destination and that destination is the seller account, anyone can broker the transaction. This is not ideal and is misleading.

Instead, with this amendment: If you set a destination, that destination needs to be the account settling the transaction. So, the broker must be the destination if they want to settle. If the buyer is the destination, then the buyer must accept the sell offer, as you cannot broker your own offers.

If users want their offers open to the public, then they should not set a destination. On the other hand, if users want to limit who can settle the offers, then they would set a destination.

Unit tests:

1. The broker cannot broker a destination offer to the buyer and the buyer must accept the sell offer. (0 transfer)
2. If the broker is the destination, the broker will take the difference. (broker mode)
This commit is contained in:
Denis Angell
2023-02-13 13:02:24 -05:00
committed by Kenny Lei
parent 39c32561bd
commit b72a87c7d3
2 changed files with 101 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp
View File

@@ -118,20 +118,40 @@ 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])
return tecNFTOKEN_BUY_SELL_MISMATCH;
// fixUnburnableNFToken
if (ctx.view.rules().enabled(fixUnburnableNFToken))
{
// 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;
}
}
// 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])
return tecNFTOKEN_BUY_SELL_MISMATCH;
// fixUnburnableNFToken
if (ctx.view.rules().enabled(fixUnburnableNFToken))
{
// 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;
}
}
// The broker can specify an amount that represents their cut; if they

113
src/test/app/NFToken_test.cpp
View File

@@ -2878,15 +2878,20 @@ 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);
{
// 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.
@@ -2923,29 +2928,52 @@ 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);
// amendment switch: When enabled the broker fails, when
// disabled the broker succeeds if the destination is the 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,
@@ -2963,15 +2991,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(
@@ -6053,4 +6086,4 @@ public:
BEAST_DEFINE_TESTSUITE_PRIO(NFToken, tx, ripple, 2);
} // namespace ripple
} // namespace ripple