fixNFTokenBrokerAccept

[dangell7]

High Level Overview of Change

Context of Change

Currently with NFTs using broker mode if the sell offer contains a destination and that destination is the buyer account, anyone can broker the transaction.

It is also true that if a buy offer contains a destination and that destination is the seller account, anyone can broker the transaction.

Imo this is not ideal and is misleading to everyone.

The comments in the code are;

// If the buyer specified a destination, that destination must be
// the seller or the broker.

This is misleading as the broker is the account submitting the transaction. Its anyone. So if the buyer specified a destination and that destination is the seller, anyone can settle the sale. I don't think that makes sense. Why even have that? If anyone can settle the transaction then there is no point in having this arithmetic.

I believe the point was; If you set a destination, that destination needs to be the person settling the transaction.

The changes made now force the broker to be the destination. If the buyer is the destination, then the buyer must accept the sell offer, as you cannot broker your own offers.

I also added a test to show one of the more recent issues. There are 2 things tested.

1. The broker cannot broker a destination offer. The buyer must accept the sell offer. (0 transfer)
2. If the broker is the destination, the broker will take the difference. (broker mode)

If users want their offers open to the public then not setting a destination would be the recommended way. Inversly, if users want to limit who can settle the offers, then they would set a destination.

Having users to both to achieve the same result is confusing imo.

XRPLF#4373

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Refactor (non-breaking change that only restructures code)
• Tests (You added tests for code that already exists, or your new feature included in this PR)
• Documentation Updates
• Release