From 1867a9edd3ca70239c7ed466353c71f459e93ce4 Mon Sep 17 00:00:00 2001 From: toteki <63419657+toteki@users.noreply.github.com> Date: Fri, 9 Sep 2022 09:19:05 -0700 Subject: [PATCH] fix: for inefficient invariants, use only in tests (#1362) ## Description closes: #1324 --- ### 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... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] added appropriate labels to the PR - [ ] targeted the correct branch (see [PR Targeting](https://github.com/umee-network/umee/blob/main/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### 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... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable) --- CHANGELOG.md | 1 + x/leverage/keeper/invariants.go | 52 +++++----------------------- x/leverage/keeper/invariants_test.go | 8 ++--- x/leverage/keeper/suite_test.go | 19 ++++++++-- 4 files changed, 30 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4e243fd0d..a2aecf968e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -112,6 +112,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ - [1300](https://github.com/umee-network/umee/pull/1300) Improve leverage test suite and error specificity. - [1322](https://github.com/umee-network/umee/pull/1322) Improve complete liquidation threshold and close factor. - [1332](https://github.com/umee-network/umee/pull/1332) Improve reserve exhaustion event and log message. +- [1362](https://github.com/umee-network/umee/pull/1362) Remove inefficent BorrowAmounts and CollateralAmounts leverage invariants. - [1363](https://github.com/umee-network/umee/pull/1332) Standardize leverage KVStore access andincrease validation. ### Bug Fixes diff --git a/x/leverage/keeper/invariants.go b/x/leverage/keeper/invariants.go index af2d60e28e..bdbb838b9b 100644 --- a/x/leverage/keeper/invariants.go +++ b/x/leverage/keeper/invariants.go @@ -19,52 +19,14 @@ const ( // RegisterInvariants registers the leverage module invariants func RegisterInvariants(ir sdk.InvariantRegistry, k Keeper) { + // these invariants run in O(N) time, with N = number of registered tokens ir.RegisterRoute(types.ModuleName, routeReserveAmount, ReserveAmountInvariant(k)) - ir.RegisterRoute(types.ModuleName, routeCollateralAmount, CollateralAmountInvariant(k)) - ir.RegisterRoute(types.ModuleName, routeBorrowAmount, BorrowAmountInvariant(k)) ir.RegisterRoute(types.ModuleName, routeBorrowAPY, BorrowAPYInvariant(k)) ir.RegisterRoute(types.ModuleName, routeSupplyAPY, SupplyAPYInvariant(k)) ir.RegisterRoute(types.ModuleName, routeInterestScalars, InterestScalarsInvariant(k)) ir.RegisterRoute(types.ModuleName, routeExchangeRates, ExchangeRatesInvariant(k)) } -// AllInvariants runs all invariants of the x/leverage module. -func AllInvariants(k Keeper) sdk.Invariant { - return func(ctx sdk.Context) (string, bool) { - res, stop := ReserveAmountInvariant(k)(ctx) - if stop { - return res, stop - } - - res, stop = CollateralAmountInvariant(k)(ctx) - if stop { - return res, stop - } - - res, stop = BorrowAmountInvariant(k)(ctx) - if stop { - return res, stop - } - - res, stop = BorrowAPYInvariant(k)(ctx) - if stop { - return res, stop - } - - res, stop = SupplyAPYInvariant(k)(ctx) - if stop { - return res, stop - } - - res, stop = InterestScalarsInvariant(k)(ctx) - if stop { - return res, stop - } - - return ExchangeRatesInvariant(k)(ctx) - } -} - // ReserveAmountInvariant checks that reserve amounts have non-negative balances func ReserveAmountInvariant(k Keeper) sdk.Invariant { return func(ctx sdk.Context) (string, bool) { @@ -109,8 +71,10 @@ func ReserveAmountInvariant(k Keeper) sdk.Invariant { } } -// CollateralAmountInvariant checks that collateral amounts have all positive values -func CollateralAmountInvariant(k Keeper) sdk.Invariant { +// InefficientCollateralAmountInvariant checks that collateral amounts have all positive values. +// This runs in O(N) time where N is the number of participating addresses, +// so it should not be enabled in production. +func InefficientCollateralAmountInvariant(k Keeper) sdk.Invariant { return func(ctx sdk.Context) (string, bool) { var ( msg string @@ -153,8 +117,10 @@ func CollateralAmountInvariant(k Keeper) sdk.Invariant { } } -// BorrowAmountInvariant checks that borrow amounts have all positive values -func BorrowAmountInvariant(k Keeper) sdk.Invariant { +// InefficientBorrowAmountInvariant checks that borrow amounts have all positive values +// This runs in O(N) time where N is the number of participating addresses, +// so it should not be enabled in production. +func InefficientBorrowAmountInvariant(k Keeper) sdk.Invariant { return func(ctx sdk.Context) (string, bool) { var ( msg string diff --git a/x/leverage/keeper/invariants_test.go b/x/leverage/keeper/invariants_test.go index a25138628b..1b7a1c5c68 100644 --- a/x/leverage/keeper/invariants_test.go +++ b/x/leverage/keeper/invariants_test.go @@ -26,7 +26,7 @@ func (s *IntegrationTestSuite) TestCollateralAmountInvariant() { s.collateralize(addr, coin("u/"+umeeDenom, 1000_000000)) // check invariant - _, broken := keeper.CollateralAmountInvariant(app.LeverageKeeper)(ctx) + _, broken := keeper.InefficientCollateralAmountInvariant(app.LeverageKeeper)(ctx) require.False(broken) uTokenDenom := types.ToUTokenDenom(appparams.BondDenom) @@ -35,7 +35,7 @@ func (s *IntegrationTestSuite) TestCollateralAmountInvariant() { s.withdraw(addr, coin(uTokenDenom, 1000_000000)) // check invariant - _, broken = keeper.CollateralAmountInvariant(app.LeverageKeeper)(ctx) + _, broken = keeper.InefficientCollateralAmountInvariant(app.LeverageKeeper)(ctx) require.False(broken) } @@ -51,7 +51,7 @@ func (s *IntegrationTestSuite) TestBorrowAmountInvariant() { s.borrow(addr, coin(appparams.BondDenom, 20_000000)) // check invariant - _, broken := keeper.BorrowAmountInvariant(app.LeverageKeeper)(ctx) + _, broken := keeper.InefficientBorrowAmountInvariant(app.LeverageKeeper)(ctx) require.False(broken) // user repays 30 umee, actually only 20 because is the min between @@ -60,6 +60,6 @@ func (s *IntegrationTestSuite) TestBorrowAmountInvariant() { require.NoError(err) // check invariant - _, broken = keeper.BorrowAmountInvariant(app.LeverageKeeper)(ctx) + _, broken = keeper.InefficientBorrowAmountInvariant(app.LeverageKeeper)(ctx) require.False(broken) } diff --git a/x/leverage/keeper/suite_test.go b/x/leverage/keeper/suite_test.go index 8132317f52..89eed1125b 100644 --- a/x/leverage/keeper/suite_test.go +++ b/x/leverage/keeper/suite_test.go @@ -208,10 +208,23 @@ func (s *IntegrationTestSuite) setReserves(coins ...sdk.Coin) { } } -// checkInvariants is used during other tests to quickly test all invariants +// checkInvariants is used during other tests to quickly test all invariants, +// including the inefficient ones we do not run in production func (s *IntegrationTestSuite) checkInvariants(msg string) { app, ctx, require := s.app, s.ctx, s.Require() - desc, broken := keeper.AllInvariants(app.LeverageKeeper)(ctx) - require.False(broken, msg, desc) + invariants := []sdk.Invariant{ + keeper.InefficientBorrowAmountInvariant(app.LeverageKeeper), + keeper.InefficientCollateralAmountInvariant(app.LeverageKeeper), + keeper.ReserveAmountInvariant(app.LeverageKeeper), + keeper.InterestScalarsInvariant(app.LeverageKeeper), + keeper.ExchangeRatesInvariant(app.LeverageKeeper), + keeper.SupplyAPYInvariant(app.LeverageKeeper), + keeper.BorrowAPYInvariant(app.LeverageKeeper), + } + + for _, inv := range invariants { + desc, broken := inv(ctx) + require.False(broken, msg, "desc", desc) + } }