From 9cd237593a38d7d28689889ff9f2ae3e0b578d35 Mon Sep 17 00:00:00 2001 From: dangell7 Date: Mon, 2 Jan 2023 17:23:25 -0500 Subject: [PATCH] fixNFTokenBrokerAccept update: error response --- src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp | 36 ++++++++++-- src/ripple/protocol/Feature.h | 3 +- src/ripple/protocol/impl/Feature.cpp | 1 + src/test/app/NFToken_test.cpp | 58 ++++++++++++++----- 4 files changed, 78 insertions(+), 20 deletions(-) diff --git a/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp b/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp index 07fe9957a76..bba775a408c 100644 --- a/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp +++ b/src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp @@ -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]) + // fixNFTokenBrokerAccept: Disabled + // the destination must be the seller or the broker. + if (!ctx.view.rules().enabled(fixNFTokenBrokerAccept) && + *dest != so->at(sfOwner) && *dest != ctx.tx[sfAccount]) + { return tecNFTOKEN_BUY_SELL_MISMATCH; + } + + // fixNFTokenBrokerAccept: Enabled + // the destination must be the tx account + if (ctx.view.rules().enabled(fixNFTokenBrokerAccept) && + *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]) + // fixNFTokenBrokerAccept: Disabled + // the destination must be the buyer or the broker. + if (!ctx.view.rules().enabled(fixNFTokenBrokerAccept) && + *dest != bo->at(sfOwner) && *dest != ctx.tx[sfAccount]) + { return tecNFTOKEN_BUY_SELL_MISMATCH; + } + + // fixNFTokenBrokerAccept: Enabled + // the destination must be the tx account + if (ctx.view.rules().enabled(fixNFTokenBrokerAccept) && + *dest != ctx.tx[sfAccount]) + { + return tecNO_PERMISSION; + } } // The broker can specify an amount that represents their cut; if they diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index d4e65a31af8..e4c6826ee19 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -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 @@ -341,6 +341,7 @@ extern uint256 const fixTrustLinesToSelf; extern uint256 const fixRemoveNFTokenAutoTrustLine; extern uint256 const featureImmediateOfferKilled; extern uint256 const featureDisallowIncoming; +extern uint256 const fixNFTokenBrokerAccept; } // namespace ripple diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 5903603f975..80f2b17292f 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -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 (fixNFTokenBrokerAccept, 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. diff --git a/src/test/app/NFToken_test.cpp b/src/test/app/NFToken_test.cpp index 42a6eb4d3ce..6ae56a9b614 100644 --- a/src/test/app/NFToken_test.cpp +++ b/src/test/app/NFToken_test.cpp @@ -2926,20 +2926,50 @@ class NFToken_test : public beast::unit_test::suite BEAST_EXPECT(ownerCount(env, minter) == 1); BEAST_EXPECT(ownerCount(env, buyer) == 2); + // fixNFTokenBrokerAccept: Disabled // 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); + if (!features[fixNFTokenBrokerAccept]) + { + 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); - // 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); + // 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; + } + else + // fixNFTokenBrokerAccept: Enabled + // Broker is not successful when destination is buyer. + // Buyer is successful with acceptOffer. + { + env(token::brokerOffers( + broker, offerMinterToBuyer, offerBuyerToMinter), + ter(tecNFTOKEN_BUY_SELL_MISMATCH)); + + 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})); + BEAST_EXPECT(ownerCount(env, issuer) == 0); + BEAST_EXPECT(ownerCount(env, minter) == 1); + BEAST_EXPECT(ownerCount(env, buyer) == 0); + } } // Show that if a buy and a sell offer both have the same destination, @@ -5079,13 +5109,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{fixNFTokenBrokerAccept}; testWithFeats(all - fixNFTDir); testWithFeats(all - disallowIncoming); + testWithFeats(all - fixNFTBroker); testWithFeats(all); } }; BEAST_DEFINE_TESTSUITE_PRIO(NFToken, tx, ripple, 2); -} // namespace ripple +} // namespace ripple \ No newline at end of file