Correct a technical flaw with NFT offers:

The existing code would, incorrectly, allow negative amounts in offers
for non-fungible tokens. Such offers would be handled very differently
depending on the context: a direct offer would fail with an error code
indicating an internal processing error, whereas brokered offers would
improperly succeed.

This commit introduces the `fixNFTokenNegOffer` amendment that detects
such offers during creation and returns an appropriate error code.

The commit also extends the existing code to allow for buy offers that
contain a `Destination` field, so that a specific broker can be set in
the offer.

src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp

@@ -75,6 +75,12 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
             if (hasExpired(ctx.view, (*offerSLE)[~sfExpiration]))
                 return {nullptr, tecEXPIRED};
 
+            // The initial implementation had a bug that allowed a negative
+            // amount.  The fixNFTokenNegOffer amendment fixes that.
+            if ((*offerSLE)[sfAmount].negative() &&
+                ctx.view.rules().enabled(fixNFTokenNegOffer))
+                return {nullptr, temBAD_OFFER};
+
             return {std::move(offerSLE), tesSUCCESS};
         }
         return {nullptr, tesSUCCESS};
@@ -103,6 +109,14 @@ 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 (auto const dest = bo->at(~sfDestination))
+        {
+            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 (auto const dest = so->at(~sfDestination))
@@ -142,6 +156,15 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
             !nft::findToken(ctx.view, ctx.tx[sfAccount], (*bo)[sfNFTokenID]))
             return tecNO_PERMISSION;
 
+        // If not in bridged mode...
+        if (!so)
+        {
+            // If the offer has a Destination field, the acceptor must be the
+            // Destination.
+            if (auto const dest = bo->at(~sfDestination);
+                dest.has_value() && *dest != ctx.tx[sfAccount])
+                return tecNO_PERMISSION;
+        }
         // The account offering to buy must have funds:
         auto const needed = bo->at(sfAmount);
 

src/ripple/app/tx/impl/NFTokenCreateOffer.cpp

@@ -46,7 +46,11 @@ NFTokenCreateOffer::preflight(PreflightContext const& ctx)
     auto const nftFlags = nft::getFlags(ctx.tx[sfNFTokenID]);
 
     {
-        auto const amount = ctx.tx[sfAmount];
+        STAmount const amount = ctx.tx[sfAmount];
+
+        if (amount.negative() && ctx.rules.enabled(fixNFTokenNegOffer))
+            // An offer for a negative amount makes no sense.
+            return temBAD_AMOUNT;
 
         if (!isXRP(amount))
         {
@@ -78,9 +82,14 @@ NFTokenCreateOffer::preflight(PreflightContext const& ctx)
 
     if (auto dest = ctx.tx[~sfDestination])
     {
-        // The destination field is only valid on a sell offer; it makes no
-        // sense in a buy offer.
-        if (!isSellOffer)
+        // Some folks think it makes sense for a buy offer to specify a
+        // specific broker using the Destination field.  This change doesn't
+        // deserve it's own amendment, so we're piggy-backing on
+        // fixNFTokenNegOffer.
+        //
+        // Prior to fixNFTokenNegOffer any use of the Destination field on
+        // a buy offer was malformed.
+        if (!isSellOffer && !ctx.rules.enabled(fixNFTokenNegOffer))
             return temMALFORMED;
 
         // The destination can't be the account executing the transaction.

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 = 48;
+static constexpr std::size_t numFeatures = 49;
 
 /** Amendments that this server supports and the default voting behavior.
    Whether they are enabled depends on the Rules defined in the validated
@@ -336,6 +336,7 @@ extern uint256 const featureCheckCashMakesTrustLine;
 extern uint256 const featureNonFungibleTokensV1;
 extern uint256 const featureExpandedSignerList;
 extern uint256 const fixNFTokenDirV1;
+extern uint256 const fixNFTokenNegOffer;
 
 }  // namespace ripple
 

src/ripple/protocol/impl/Feature.cpp

@@ -431,7 +431,7 @@ REGISTER_FEATURE(RequireFullyCanonicalSig,      Supported::yes, DefaultVote::yes
 REGISTER_FIX    (fix1781,                       Supported::yes, DefaultVote::yes);
 REGISTER_FEATURE(HardenedValidations,           Supported::yes, DefaultVote::yes);
 REGISTER_FIX    (fixAmendmentMajorityCalc,      Supported::yes, DefaultVote::yes);
-REGISTER_FEATURE(NegativeUNL,                   Supported::yes, DefaultVote::no);
+REGISTER_FEATURE(NegativeUNL,                   Supported::yes, DefaultVote::yes);
 REGISTER_FEATURE(TicketBatch,                   Supported::yes, DefaultVote::yes);
 REGISTER_FEATURE(FlowSortStrands,               Supported::yes, DefaultVote::yes);
 REGISTER_FIX    (fixSTAmountCanonicalize,       Supported::yes, DefaultVote::yes);
@@ -440,6 +440,7 @@ REGISTER_FEATURE(CheckCashMakesTrustLine,       Supported::yes, DefaultVote::no)
 REGISTER_FEATURE(NonFungibleTokensV1,           Supported::yes, DefaultVote::no);
 REGISTER_FEATURE(ExpandedSignerList,            Supported::yes, DefaultVote::no);
 REGISTER_FIX    (fixNFTokenDirV1,               Supported::yes, DefaultVote::no);
+REGISTER_FIX    (fixNFTokenNegOffer,            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.