-
Notifications
You must be signed in to change notification settings - Fork 170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add DirectLiquidationFee and uToken liquidation option #1222
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adds proto for DirectLiquidationFee
as well as actually implementing the behavior.
Misc additions:
- Moved token <-> uToken denom functions to
types
package. Previously there were similar functions in bothtypes
andkeeper
. - Improved operations tests in simulation package and fixed a hidden bug (fees making messages fail if the random generator hit certain values, due to some transactions not using
CoinsSpentInMsg
)
Codecov Report
@@ Coverage Diff @@
## main #1222 +/- ##
==========================================
- Coverage 51.20% 50.97% -0.24%
==========================================
Files 67 67
Lines 6657 6772 +115
==========================================
+ Hits 3409 3452 +43
- Misses 3008 3077 +69
- Partials 240 243 +3
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good! liquidation logic change is sound & simple. mostly just have nitpics 😄
Co-authored-by: Adam Wozniak <29418299+adamewozniak@users.noreply.github.com>
Co-authored-by: Adam Wozniak <29418299+adamewozniak@users.noreply.github.com>
Co-authored-by: Adam Wozniak <29418299+adamewozniak@users.noreply.github.com>
Co-authored-by: Adam Wozniak <29418299+adamewozniak@users.noreply.github.com>
Co-authored-by: Adam Wozniak <29418299+adamewozniak@users.noreply.github.com>
if err != nil { | ||
return err | ||
} | ||
return k.setBorrow(ctx, borrowAddr, k.GetBorrow(ctx, borrowAddr, repay.Denom).Sub(repay)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if result is zero (old_borrow - repay
) then we should close the position and remove the record.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setBorrow
(or technically setAdjustedBorrow
which it uses internally) clears the record whenever something would be set to zero
// If there are not enough uTokens in balance, Withdraw will attempt to withdraw uToken collateral | ||
// to make up the difference (as long as borrow limit allows). If the uToken denom is invalid or | ||
// balances are insufficient to withdraw the full amount requested, returns an error. | ||
func (k Keeper) Withdraw(ctx sdk.Context, supplierAddr sdk.AccAddress, uToken sdk.Coin) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for renaming to uToken
Description
DirectLiquidationFee
as well as actually implementing the behavior.Misc additions:
types
package. Previously there were similar functions in bothtypes
andkeeper
.CoinsSpentInMsg
)closes: #1158
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...