-
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!: liquidation directly rewards base assets #1118
Merged
Merged
Changes from 53 commits
Commits
Show all changes
71 commits
Select commit
Hold shift + click to select a range
828d73d
featgit statusiquidation directly rewards base assets
toteki 5721ff2
Merge branch 'main' into adam/liquidate
toteki 025f017
make proto-gen
toteki 51f34dd
changelog++
toteki b15cc7e
comment++
toteki e129f3c
spec++
toteki 7513e98
whitespace--
toteki 5c61a56
function rename
toteki bcb555f
add MsgLiquidate response field for collateral consumed
toteki 1ef4e90
line length--
toteki 19d3f07
Merge branch 'main' into adam/liquidate
toteki dcb30d8
Merge branch 'main' into adam/liquidate
toteki e0aa350
update comment and rename return values
toteki c88119c
Merge branch 'main' into adam/liquidate
toteki f498ef4
make proto-gen
toteki 73509d8
Merge branch 'main' into adam/liquidate
toteki 035fb53
++
toteki 3145f57
suggestion++
toteki 654e6ed
suggestion++
toteki a81116c
Merge branch 'main' into adam/liquidate
toteki 64d8e74
Merge branch 'main' into adam/liquidate
toteki 5ee767b
comment++
toteki e0485c9
Merge branch 'adam/liquidate' of github.com:umee-network/umee into
toteki 59a058f
move an error between functions
toteki fd21435
fix test
toteki 9ccab78
add descriptive strings to math test cases
toteki 144993e
proto comments
toteki 03d0e08
filter blacklisted collateral before checking for bad debt
toteki 27b9500
Merge branch 'main' into adam/liquidate
toteki 4e021d9
Merge branch 'main' into adam/liquidate
toteki af2c201
Merge branch 'main' into adam/liquidate
robert-zaremba 93cf56c
separate out liquidation function and reduce rounding
toteki d27f904
clarify math comments
toteki 9713fff
clarify math comments
toteki 3331a00
more test cases and a rounding TODO
toteki c007643
++
toteki 8069c57
Merge branch 'main' into adam/liquidate
RafilxTenfen 08f405a
Merge branch 'main' into adam/liquidate
toteki 865c5d3
refactor and add detailed reasoning to liquidate computation
toteki c2ceae9
final test cases
toteki 47113cd
remove redundant zero-price checks in PriceRatio
toteki 1dea021
--
toteki ee01003
move liquidate computations back to keeper package
toteki 9003703
move liquidate computations back to keeper package
toteki 32ba636
Merge branch 'main' into adam/liquidate
toteki 1ecaff8
Merge branch 'main' into adam/liquidate
toteki 3f0e9b1
Merge branch 'main' into adam/liquidate
toteki f0fbba1
Merge branch 'main' into adam/liquidate
toteki ebab749
Merge branch 'main' into adam/liquidate
toteki f31b793
Merge branch 'main' into adam/liquidate
toteki 93a24bb
Merge branch 'main' into adam/liquidate
toteki c4ed80f
Merge branch 'main' into adam/liquidate
toteki 5d425ce
Merge branch 'main' into adam/liquidate
toteki 7696a85
suggestion++
toteki b4113aa
make proto-gen
toteki b018703
remove ctx from filterCoins
toteki 891c24b
Merge branch 'main' into adam/liquidate
toteki 3b1a3a0
long example liquidate command
toteki c6aab01
Merge branch 'adam/liquidate' of github.com:umee-network/umee into ad…
toteki c0ee072
Merge branch 'main' into adam/liquidate
toteki ed87cb0
package keeper_test -> keeper for math unit tests
toteki dacfe9f
suggestion++
toteki 4b1f543
suggestion++
toteki 4b7bd3f
Merge branch 'main' into adam/liquidate
toteki 11432bf
fix test amounts
toteki 50abc33
update CloseFactor comment
toteki 8542b4c
Merge branch 'main' into adam/liquidate
toteki 0ed0234
++
toteki df91850
++
toteki 28e905c
Merge branch 'main' into adam/liquidate
toteki 3169527
Merge branch 'main' into adam/liquidate
mergify[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -296,17 +296,7 @@ func (s *IntegrationTestSuite) TestLeverageScenario() { | |
val.Address.String(), | ||
val.Address.String(), | ||
"5uumee", | ||
"4uumee", | ||
}, | ||
nil, | ||
} | ||
|
||
fixCollateral := testTransaction{ | ||
"add back collateral received from liquidation", | ||
cli.GetCmdCollateralize(), | ||
[]string{ | ||
val.Address.String(), | ||
"4u/uumee", | ||
"uumee", | ||
}, | ||
nil, | ||
} | ||
|
@@ -316,7 +306,7 @@ func (s *IntegrationTestSuite) TestLeverageScenario() { | |
cli.GetCmdRepay(), | ||
[]string{ | ||
val.Address.String(), | ||
"51uumee", | ||
"50uumee", | ||
Comment on lines
-189
to
+179
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changes in rounding behavior have created a number of simpler decimals in this test file. |
||
}, | ||
nil, | ||
} | ||
|
@@ -326,7 +316,7 @@ func (s *IntegrationTestSuite) TestLeverageScenario() { | |
cli.GetCmdDecollateralize(), | ||
[]string{ | ||
val.Address.String(), | ||
"1000u/uumee", | ||
"950u/uumee", | ||
}, | ||
nil, | ||
} | ||
|
@@ -336,7 +326,7 @@ func (s *IntegrationTestSuite) TestLeverageScenario() { | |
cli.GetCmdWithdraw(), | ||
[]string{ | ||
val.Address.String(), | ||
"1000u/uumee", | ||
"950u/uumee", | ||
}, | ||
nil, | ||
} | ||
|
@@ -352,7 +342,7 @@ func (s *IntegrationTestSuite) TestLeverageScenario() { | |
&types.QuerySuppliedResponse{}, | ||
&types.QuerySuppliedResponse{ | ||
Supplied: sdk.NewCoins( | ||
sdk.NewInt64Coin(umeeapp.BondDenom, 1001), | ||
sdk.NewInt64Coin(umeeapp.BondDenom, 1000), | ||
), | ||
}, | ||
}, | ||
|
@@ -367,7 +357,7 @@ func (s *IntegrationTestSuite) TestLeverageScenario() { | |
&types.QuerySuppliedResponse{}, | ||
&types.QuerySuppliedResponse{ | ||
Supplied: sdk.NewCoins( | ||
sdk.NewInt64Coin(umeeapp.BondDenom, 1001), | ||
sdk.NewInt64Coin(umeeapp.BondDenom, 1000), | ||
), | ||
}, | ||
}, | ||
|
@@ -381,7 +371,7 @@ func (s *IntegrationTestSuite) TestLeverageScenario() { | |
&types.QueryBorrowedResponse{}, | ||
&types.QueryBorrowedResponse{ | ||
Borrowed: sdk.NewCoins( | ||
sdk.NewInt64Coin(umeeapp.BondDenom, 47), | ||
sdk.NewInt64Coin(umeeapp.BondDenom, 51), | ||
), | ||
}, | ||
}, | ||
|
@@ -396,7 +386,7 @@ func (s *IntegrationTestSuite) TestLeverageScenario() { | |
&types.QueryBorrowedResponse{}, | ||
&types.QueryBorrowedResponse{ | ||
Borrowed: sdk.NewCoins( | ||
sdk.NewInt64Coin(umeeapp.BondDenom, 47), | ||
sdk.NewInt64Coin(umeeapp.BondDenom, 51), | ||
), | ||
}, | ||
}, | ||
|
@@ -441,8 +431,8 @@ func (s *IntegrationTestSuite) TestLeverageScenario() { | |
// From app/test_helpers.go/IntegrationTestNetworkConfig | ||
// This result is umee's oracle exchange rate times the | ||
// amount supplied. | ||
SuppliedValue: sdk.MustNewDecFromStr("0.03424421"), | ||
// (1001 / 1000000) umee * 34.21 = 0.03424421 | ||
SuppliedValue: sdk.MustNewDecFromStr("0.03421"), | ||
// (1000 / 1000000) umee * 34.21 = 0.034241 | ||
}, | ||
}, | ||
testQuery{ | ||
|
@@ -456,8 +446,8 @@ func (s *IntegrationTestSuite) TestLeverageScenario() { | |
&types.QuerySuppliedValueResponse{}, | ||
&types.QuerySuppliedValueResponse{ | ||
// From app/test_helpers.go/IntegrationTestNetworkConfig | ||
SuppliedValue: sdk.MustNewDecFromStr("0.03424421"), | ||
// (1001 / 1000000) umee * 34.21 = 0.03424421 | ||
SuppliedValue: sdk.MustNewDecFromStr("0.03421"), | ||
// (1000 / 1000000) umee * 34.21 = 0.03421 | ||
}, | ||
}, | ||
testQuery{ | ||
|
@@ -470,8 +460,8 @@ func (s *IntegrationTestSuite) TestLeverageScenario() { | |
&types.QueryCollateralValueResponse{}, | ||
&types.QueryCollateralValueResponse{ | ||
// From app/test_helpers.go/IntegrationTestNetworkConfig | ||
CollateralValue: sdk.MustNewDecFromStr("0.03424421"), | ||
// (1001 / 1000000) umee * 34.21 = 0.03424421 | ||
CollateralValue: sdk.MustNewDecFromStr("0.03421"), | ||
// (1001 / 1000000) umee * 34.21 = 0.03421 | ||
}, | ||
}, | ||
testQuery{ | ||
|
@@ -484,8 +474,8 @@ func (s *IntegrationTestSuite) TestLeverageScenario() { | |
false, | ||
&types.QueryCollateralValueResponse{}, | ||
&types.QueryCollateralValueResponse{ | ||
CollateralValue: sdk.MustNewDecFromStr("0.03424421"), | ||
// (1001 / 1000000) umee * 34.21 = 0.03424421 | ||
CollateralValue: sdk.MustNewDecFromStr("0.03421"), | ||
// (1000 / 1000000) umee * 34.21 = 0.03421 | ||
}, | ||
}, | ||
testQuery{ | ||
|
@@ -498,8 +488,8 @@ func (s *IntegrationTestSuite) TestLeverageScenario() { | |
&types.QueryBorrowedValueResponse{}, | ||
&types.QueryBorrowedValueResponse{ | ||
// From app/test_helpers.go/IntegrationTestNetworkConfig | ||
BorrowedValue: sdk.MustNewDecFromStr("0.00160787"), | ||
// (51 / 1000000) umee * 34.21 = 0.00160787 | ||
BorrowedValue: sdk.MustNewDecFromStr("0.00174471"), | ||
// (51 / 1000000) umee * 34.21 = 0.00174471 | ||
}, | ||
}, | ||
testQuery{ | ||
|
@@ -513,8 +503,8 @@ func (s *IntegrationTestSuite) TestLeverageScenario() { | |
&types.QueryBorrowedValueResponse{}, | ||
&types.QueryBorrowedValueResponse{ | ||
// From app/test_helpers.go/IntegrationTestNetworkConfig | ||
BorrowedValue: sdk.MustNewDecFromStr("0.00160787"), | ||
// (51 / 1000000) umee * 34.21 = 0.00160787 | ||
BorrowedValue: sdk.MustNewDecFromStr("0.00174471"), | ||
// (51 / 1000000) umee * 34.21 = 0.00174471 | ||
}, | ||
}, | ||
testQuery{ | ||
|
@@ -527,8 +517,8 @@ func (s *IntegrationTestSuite) TestLeverageScenario() { | |
&types.QueryBorrowLimitResponse{}, | ||
&types.QueryBorrowLimitResponse{ | ||
// From app/test_helpers.go/IntegrationTestNetworkConfig | ||
BorrowLimit: sdk.MustNewDecFromStr("0.0017122105"), | ||
// (1001 / 1000000) * 0.05 * 34.21 = 0.0017122105 | ||
BorrowLimit: sdk.MustNewDecFromStr("0.0017105"), | ||
// (1000 / 1000000) * 0.05 * 34.21 = 0.0017105 | ||
}, | ||
}, | ||
testQuery{ | ||
|
@@ -541,8 +531,8 @@ func (s *IntegrationTestSuite) TestLeverageScenario() { | |
&types.QueryLiquidationThresholdResponse{}, | ||
&types.QueryLiquidationThresholdResponse{ | ||
// From app/test_helpers.go/IntegrationTestNetworkConfig | ||
LiquidationThreshold: sdk.MustNewDecFromStr("0.0017122105"), | ||
// (1001 / 1000000) * 0.05 * 34.21 = 0.0017122105 | ||
LiquidationThreshold: sdk.MustNewDecFromStr("0.0017105"), | ||
// (1000 / 1000000) * 0.05 * 34.21 = 0.0017105 | ||
}, | ||
}, | ||
} | ||
|
@@ -555,17 +545,16 @@ func (s *IntegrationTestSuite) TestLeverageScenario() { | |
supply, | ||
addCollateral, | ||
borrow, | ||
liquidate, | ||
fixCollateral, | ||
) | ||
|
||
// These transactions are deferred to run after nonzero queries are finished | ||
defer s.runTestCases( | ||
// These queries run while the supplying and borrowing is active to produce nonzero output | ||
s.runTestCases(nonzeroQueries...) | ||
|
||
// These transactions run after nonzero queries are finished | ||
s.runTestCases( | ||
liquidate, | ||
repay, | ||
removeCollateral, | ||
withdraw, | ||
) | ||
|
||
// These queries run while the supplying and borrowing is active to produce nonzero output | ||
s.runTestCases(nonzeroQueries...) | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package keeper | ||
|
||
import ( | ||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
) | ||
|
||
// filterCoins returns the subset of an sdk.Coins that meet a given condition | ||
func (k Keeper) filterCoins(ctx sdk.Context, coins sdk.Coins, accept func(sdk.Coin) bool) sdk.Coins { | ||
toteki marked this conversation as resolved.
Show resolved
Hide resolved
|
||
filtered := sdk.Coins{} | ||
for _, c := range coins { | ||
if accept(c) { | ||
filtered = append(filtered, c) | ||
} | ||
} | ||
return filtered | ||
} | ||
|
||
// filterAcceptedCoins returns the subset of an sdk.Coins that are accepted, non-blacklisted tokens | ||
func (k Keeper) filterAcceptedCoins(ctx sdk.Context, coins sdk.Coins) sdk.Coins { | ||
return k.filterCoins( | ||
ctx, | ||
coins, | ||
func(c sdk.Coin) bool { | ||
return k.validateAcceptedAsset(ctx, c) == nil | ||
}, | ||
) | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
We need to think more about this change and this PR. In particular, I think this should be
repeated Coin
, see: #1129There 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.
Considering the level of mathematical complexity that single-denom liquidations involve (see below), I prefer avoiding
sdk.Coins
in either repayment or reward.It does increase the number of
MsgLiquidate
required for complex positions unfortunately, but keeps the edge cases much more understandable.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.
In that case the complexity is moved to liquidator... he will need to calculate which denom he can liquidate and how much.
We are already adjusting the repayment. Instead we can go one by one of the collateral list and sum up to the value related to the repayment. We could use denom list to make it simpler.
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.
Yes, the liquidator can do that - I had a script doing as much during the testnet, and what it does is proceed in order of preference for repay and reward denoms by offering the maximum amount it is willing to repay.
For most borrowers, even with multiple borrowed and collateral types, the first
MsgLiquidate
brings them back to health, so the liquidator's initial preference for reward and repay denoms is enough.For borrowers where multiple transactions are needed, the liquidator only needs to follow their ranked order of preferences, because the first transaction will have either exhausted a repay denom or a reward denom by succeeding.
Note: Exhausted a denom can mean:
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.
Do we have a test verifying if the liquidation of a single asset, even if it doesn't bring the account to a healthy state, is possible?
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.
Liquidation logic is independent of final state (except for the collateral == 0 check for bad debt flagging) so that isn't a problem.
In particular, the test case labeled
repayLimited
covers this scenario inliquidate_test.go