0028 XLS-28d: Fix for negative spreads and blocked books

[Mwni]

0028 XLS-28d: Fix for negative spreads and blocked books

The Issue

Under certain circumstances, Offers, when almost completely consumed, have a much lower exchange rate than when they were first placed. This occurs when the remaining amounts of one or both assets are so small that they cannot be rounded to a similar ratio as when the Offer was placed.

How this causes negative spreads:

When a Offer enters this offending state, it "hides" other legitimate offers with superior exchange rate behind it. New limit Offers on the counter side are no longer able to cross and consume said legitimate Offers and instead end up in the book, if they are not killed by the makers preference. These new Offers trend to pile up and remain there, even after the offending offer is removed.

Previous Work

This issue is known. The amendment fixRmSmallIncreasedQOffers aims to resolve this, and does so, very well in most of the cases. However, this initial amendment only affects offers with a remaining funded amount that is less or equal to the smallest atomic unit of the traded currency. This is problematic, because there is still a lot room for rounding errors for values greater than 1.

The chart below illustrates this.

This data was obtained by repeatedly creating an offending Offer with varying fractional funded TakerPays values (X axis) and then testing how far above the apparent exchange rate (Y axis) an opposing Offer has to be, in order to get filled. The range between 0 - 1 drops marked in yellow shows the efficacy of the current fixRmSmallIncreasedQOffers amendment.

The fix

The fix is staightforward and was first suggested by @RichardAH. We simply increase the minimum value for the existing fixRmSmallIncreasedQOffers logic to consider the Offer as potentially offending. The choice for the new limit is open for discussion, and can be adjusted here. As recommended by Richard, 1,000 drops ensure the margin of error is kept below 0.1%.

I propose a new amendment called fixNegativeSpreadsV1 that implements this minimal change, while utilizing the existing logic.

Testing it

There is a small script that automatically sets up a book, creates an offending Offer, and then checks its real exchange rate, either on testnet or on your own standalone node.

[Cadey]

Hi @Mwni thanks for all you investigating on this issue, I have personally had this happen to a token.

There is another issue however that in some ways is much more of a concern, if still open.

When an order book is showing a negative spread a buyer is able to make an offer high into the sell side and the order would complete. I did send some examples of this to ripple back in Nov and I'll see if I can find them.

Since you can recreate the negative spread issue on demand now, try making an offer deep into the sell side and see if it still triggers bypassing the sell offers lower than it

[Cadey]

This was my original bug report, it's not worded all that well as I re read it

XRPLF/rippled#3985

But essentially when you get a negative spreed, setup a buy order higher than the sell price, then do a sell order into it. It was matching even though it should have matched the other sell orders...

[Cadey]

I'll see if using your script to bork an order book allows me to do it and I'll document it better :)

[Mwni]

Okay, yes, that is indeed possible, but it's different than being literally able to skip offers before consuming. These newly created limit offers that are contributing to a negative spread are consumable, but only by their specified exchange rate. At no point is some party actually getting an unfair rate.

[Cadey]

Well I guess it could be unfair to people who are selling already. Let's say the lowest sell is 50, and we have a negative spread of - 50, that means there is an opne buy order at 100. Ow if your looking to sell you can sell directly into an offer of 100 and the other guy selling at 50 kinder looses out since there sell was technically the next one to trigger

[Mwni]

Yes, I agree. This issue can deceive people into placing unfavorable offers, but it's not the same as offers being filled for a worse exchange rate than specified when the offer was created.

[mvadari]

@Mwni should this spec be moved into PR and kept in the repo or should it be closed? Note that this does not mean that the spec has been accepted/implemented/finalized.

I will move it to PR by copy-pasting the existing draft if there is no response in the next week.

[Mwni]

Not entirely sure. The outlined issue has been fixed in its entirety by @seelabs fixReducedOffersV1 amendment. The proposed solution in this draft has not been implemented.

[Tapanito]

Closing this discussion because the issue has been fixed.