From 702526e28d946bf74aa6c786ba9a2d0d893bb773 Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Thu, 24 Mar 2022 14:05:37 -0700 Subject: [PATCH] [FOLD] Allow an NFT sell offer's destination to be a broker --- src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp | 15 ++-- src/test/app/NFToken_test.cpp | 72 ++++++++++++++----- 2 files changed, 64 insertions(+), 23 deletions(-) diff --git a/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp b/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp index fc46f2831bd..b06edcf5d81 100644 --- a/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp +++ b/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp @@ -88,7 +88,8 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx) return ret; if (buy && sell) - { // Brokered mode: + { + // Brokered mode: auto const bo = ctx.view.read(keylet::nftoffer(*buy)); auto const so = ctx.view.read(keylet::nftoffer(*sell)); @@ -106,10 +107,10 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx) return tecINSUFFICIENT_PAYMENT; // If the seller specified a destination, that destination must be - // the buyer. + // the buyer or the broker. if (auto const dest = so->at(~sfDestination)) { - if (*dest != bo->at(sfOwner)) + if (*dest != bo->at(sfOwner) && *dest != ctx.tx[sfAccount]) return tecNFTOKEN_BUY_SELL_MISMATCH; } @@ -306,10 +307,10 @@ NFTokenAcceptOffer::doApply() // // It is important that the issuer's cut be calculated after the // broker's portion is already removed. Calculating the issuer's - // cut before the broker's cut it removed can result in more money + // cut before the broker's cut is removed can result in more money // being paid out than the seller authorized. That would be bad! - // Send the broker the amount they requested + // Send the broker the amount they requested. if (auto const cut = ctx_.tx[~sfBrokerFee]; cut && cut.value() != beast::zero) { @@ -320,7 +321,7 @@ NFTokenAcceptOffer::doApply() amount -= cut.value(); } - // Calculate the issuer's cut, if any: + // Calculate the issuer's cut, if any. if (auto const fee = nft::getTransferFee(tokenID); amount != beast::zero && fee != 0) { @@ -336,7 +337,7 @@ NFTokenAcceptOffer::doApply() } } - // And send whatever remains to the seller + // And send whatever remains to the seller. if (amount > beast::zero) { if (auto const r = pay(buyer, seller, amount); !isTesSuccess(r)) diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 16fb8d04b17..1e229e5bf89 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -2649,8 +2649,9 @@ class NFToken_test : public beast::unit_test::suite Account const issuer{"issuer"}; Account const minter{"minter"}; Account const buyer{"buyer"}; + Account const broker{"broker"}; - env.fund(XRP(1000), issuer, minter, buyer); + env.fund(XRP(1000), issuer, minter, buyer, broker); // We want to explore how issuers vs minters fits into the permission // scheme. So issuer issues and minter mints. @@ -2775,14 +2776,13 @@ class NFToken_test : public beast::unit_test::suite BEAST_EXPECT(ownerCount(env, buyer) == 0); } - // Show that brokered mode cannot complete a transfer where the - // Destination doesn't match, but can complete if the Destination - // does match. + // Show that a sell offer's Destination can broker that sell offer + // to another account. { - uint256 const offerMinterToBuyer = + uint256 const offerMinterToBroker = keylet::nftoffer(minter, env.seq(minter)).key; env(token::createOffer(minter, tokenID, drops(1)), - token::destination(buyer), + token::destination(broker), txflags(tfSellToken)); uint256 const offerBuyerToMinter = @@ -2790,32 +2790,72 @@ class NFToken_test : public beast::unit_test::suite env(token::createOffer(buyer, tokenID, drops(1)), token::owner(minter)); - uint256 const offerIssuerToMinter = + 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. + 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); + + // Since broker is the sell offer's destination, they can broker + // the two offers. + env(token::brokerOffers( + broker, offerBuyerToMinter, offerMinterToBroker)); + env.close(); + BEAST_EXPECT(ownerCount(env, issuer) == 0); + BEAST_EXPECT(ownerCount(env, minter) == 0); + BEAST_EXPECT(ownerCount(env, buyer) == 1); + } + + // Show that brokered mode cannot complete a transfer where the + // Destination doesn't match, but can complete if the Destination + // does match. + { + uint256 const offerBuyerToMinter = + keylet::nftoffer(buyer, env.seq(buyer)).key; + env(token::createOffer(buyer, tokenID, drops(1)), + token::destination(minter), + txflags(tfSellToken)); + + uint256 const offerMinterToBuyer = + keylet::nftoffer(minter, env.seq(minter)).key; + env(token::createOffer(minter, tokenID, drops(1)), + token::owner(buyer)); + + uint256 const offerIssuerToBuyer = keylet::nftoffer(issuer, env.seq(issuer)).key; env(token::createOffer(issuer, tokenID, drops(1)), - token::owner(minter)); + token::owner(buyer)); env.close(); BEAST_EXPECT(ownerCount(env, issuer) == 1); - BEAST_EXPECT(ownerCount(env, minter) == 2); - BEAST_EXPECT(ownerCount(env, buyer) == 1); + 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( - buyer, offerIssuerToMinter, offerMinterToBuyer), + broker, offerIssuerToBuyer, offerBuyerToMinter), ter(tecNFTOKEN_BUY_SELL_MISMATCH)); env.close(); BEAST_EXPECT(ownerCount(env, issuer) == 1); - BEAST_EXPECT(ownerCount(env, minter) == 2); - BEAST_EXPECT(ownerCount(env, buyer) == 1); + BEAST_EXPECT(ownerCount(env, minter) == 1); + BEAST_EXPECT(ownerCount(env, buyer) == 2); // Broker is successful when destination is buyer. env(token::brokerOffers( - issuer, offerBuyerToMinter, offerMinterToBuyer)); + broker, offerMinterToBuyer, offerBuyerToMinter)); env.close(); BEAST_EXPECT(ownerCount(env, issuer) == 1); - BEAST_EXPECT(ownerCount(env, minter) == 0); - BEAST_EXPECT(ownerCount(env, buyer) == 1); + BEAST_EXPECT(ownerCount(env, minter) == 1); + BEAST_EXPECT(ownerCount(env, buyer) == 0); } }