diff --git a/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp b/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp index 257bda5c051..c420bfc6197 100644 --- a/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp +++ b/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp @@ -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 diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index a82c50e842a..27ac98f1530 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -2872,15 +2872,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. @@ -2917,29 +2922,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, @@ -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( @@ -6047,4 +6080,4 @@ class NFToken_test : public beast::unit_test::suite BEAST_DEFINE_TESTSUITE_PRIO(NFToken, tx, ripple, 2); -} // namespace ripple +} // namespace ripple \ No newline at end of file