THORChain - Enforce MinOut in borrows

[gomesalexandre]

Overview

Currently, we have no MinOut in borrows, i.e empty limit component. That's actually as returned by us by THOR, see their example memo in the Swagger docs: $+:ETH.ETH:0x1c7b17362c84287bd1184447e6dfeaf920c31bbe.

Skip to the bottom for the tl;dr.

Now, in theory, a limit should be able to be added, as per all THOR "deposit" memos: https://dev.thorchain.org/concepts/memos.html#open-loan

:MINOUT | Minimum debt amount, else a refund. Similar to :LIM. | Optional. 1e8 format.

So all we need to do is add the returned expected_amount_out from the quote as a limit and call it a day, right? ...Right? Wrong. In reality, things get much, much weirder.

There is an undocumented behavior of THOR defaulting to internal streaming swaps with an absence of limit component. The expected_amount_out we're getting is assuming we use the returned memo (which doesn't contain a limit) as-is, and is more for informational purposes than anything else.

Trying to add said expected_amount_out in our memo will result in failures: https://viewblock.io/thorchain/tx/d76ed393598e60ec6d1aef287023c950223fa5db59f46250b7c5754cd9ea3644

The reason is this makes the loan a non-streaming loan, with slightly worsened out amount, and so the initial expected_amount_out returned to us doesn't cut it anymore.

There is also another component of confusion here: is fees.slippage_bps ever our guy as far as fees our concerned, i.e if circling back to the original issue which prompted us to enforce MinOut here (a user getting a very, very bad rate with a specific token as a borrow asset on Avalanche), would fees.slippage_bps have caught that?

If so, we may not need further work here at all and should be able to keep relying on the empty limit component, benefiting from streaming swaps, now that <WarningAcknwledgement /> is implemented for borrows:

https://github.com/shapeshift/web/blob/537f6b872a5a871a17fb393ed810ab61d13b4616/src/pages/Lending/Pool/components/Borrow/BorrowInput.tsx#L492-L497

Sure, we could add a limit in the memo for extra safety, but it wouldn't do much, as we'd really be adding slippage of the streaming loan expected_amount_out (already all fees deducted, including slippage) accounting for the worsening of quote and would really be guarding against things being even worse than originally quoted at execution time, not against high slippage.

tl;dr what are we solving here, is this user safety at execution time? If we can't make users safer at execution time (and probably shouldn't, as they acknowledged high slippage already), and we now have UX/UI to properly warn them, which is how they ended up in such a situation to begin with, is there still value in adding a limit that doesn't do much in terms of protection?

References and additional details

• <WarningAcknowledgement /> implementation for loan opens:

https://github.com/shapeshift/web/blob/537f6b872a5a871a17fb393ed810ab61d13b4616/src/pages/Lending/Pool/components/Borrow/BorrowInput.tsx#L492-L497

• assertAndProcessMemo for loan opens in develop currently:

https://github.com/shapeshift/web/blob/537f6b872a5a871a17fb393ed810ab61d13b4616/src/lib/utils/thorchain/memo/assertAndProcessMemo.ts#L162-L163

Acceptance Criteria

Either:

• We are able to implement reliable MinOut which is actually protecting users
• We confirm that fees.slippage_bps reflects high-slippage situations, close this as wontfix, and add an extra comment to all the currently commented out bits to clearly mention we should never uncomment it as this will result in refunds

Need By Date

No response

Screenshots/Mockups

No response

Estimated effort

No response