Liquidations

[0xdcota]

@ben132333 please check the comments I added above.

Also, I believe I missed to mention but in this repository we are trying to following a standard way for our commit messages. Please refer to the Commit message style here

[brozorec]

@DaigaroCota can you add a section in the general doc on how liquidation works. Also, maybe some more comments in the contract about it may be helpful.

[brozorec]

@DaigaroCota @ben132333

• How do we use liqRatio when performing a liquidation?
• According to your implementation the liquidator can do a single liquidation at a time and at the end is left with shares in the vault. This gives enough flexibility but we have to figure out a way to allow for multiple liquidations and usage of flashloans. I see these features fit well if we use the router. Wdyt?

[0xdcota]

I was thinking we do not over-complicate the function in the BorrowingVault, so yes the intention was there to just keep it simple. For me we can either create a separate contract or we include in the router. Though, my only comment would be that we need to check how big in bytecode with optimization runs is getting. Perhaps also in terms of access to the liquidation function it becomes cleaner on a seperate contract? That way we can include helper functions that can determine if a user is insolvent (profit-less liquidations) before execution of an array of users.

[ben132333]

The liqRatio variable is not used when performing a liquidation. I was unaware of it. The healthFactor determines + determineLiquidatorFactor determine when a user can be liquidated. Should liqRatio be integrated?

[0xdcota]

The liqRatio variable is not used when performing a liquidation. I was unaware of it. The healthFactor determines + determineLiquidatorFactor determine when a user can be liquidated. Should liqRatio be integrated?

This was a miss on my side, but it is an easy fix, and I will take care of it. Essentially the maxLTV value needs to be replaced by liqRatio. This is to avoid users that take a debt position for the first time and decide to borrow the maximum amount, basically expose themselves to instant liquidation. That is why liqRatio should be > maxLTV with some margin in between. This allows at least a window between having maxed out borrowing amount and the possibility of liquidation.

[brozorec]

I was thinking we do not over-complicate the function in the BorrowingVault, so yes the intention was there to just keep it simple. For me we can either create a separate contract or we include in the router. Though, my only comment would be that we need to check how big in bytecode with optimization runs is getting. Perhaps also in terms of access to the liquidation function it becomes cleaner on a seperate contract? That way we can include helper functions that can determine if a user is insolvent (profit-less liquidations) before execution of an array of users.

I agree with this. However, I suggest we can prepare the ground for it here. If we want this function to be called by another contract (the router or another one), we need at least one new input param to identify the receiver, the same way deposit and payback work:

liquidate(address owner, address receiver) {
  ...
  address _receiver = receiver != address(0) ? receiver : caller;
  mint(_receiver, shares);
}

[0xdcota]

I was thinking we do not over-complicate the function in the BorrowingVault, so yes the intention was there to just keep it simple. For me we can either create a separate contract or we include in the router. Though, my only comment would be that we need to check how big in bytecode with optimization runs is getting. Perhaps also in terms of access to the liquidation function it becomes cleaner on a seperate contract? That way we can include helper functions that can determine if a user is insolvent (profit-less liquidations) before execution of an array of users.

I agree with this. However, I suggest we can prepare the ground for it here. If we want this function to be called by another contract (the router or another one), we need at least one new input param to identify the receiver, the same way deposit and payback work:

liquidate(address owner, address receiver) {
  ...
  address _receiver = receiver != address(0) ? receiver : caller;
  mint(_receiver, shares);
}

Implemented as suggested, only difference is that I forced receiver to never be address zero. If you are liquidating, either you pass yourself (address) as receiver or someone else (even if "you" is a contract).