Skip to content
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

test(vpool): Lots of testing for 'nibid q vpool all-pools'. #830

Merged
merged 9 commits into from
Aug 19, 2022
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Improvements

* [#830](https://github.com/NibiruChain/nibiru/pull/830) - test(vpool): Make missing fields for 'query vpool all-pools' display as empty strings.
- Improve test coverage of functions used in the query server.
- Added 'pair' field to the `all-pools` to make the prices array easier to digest
* [#798](https://github.com/NibiruChain/nibiru/pull/798) - fix integration tests caused by PR #786
* [#801](https://github.com/NibiruChain/nibiru/pull/801) - remove unused pair constants
* [#788](https://github.com/NibiruChain/nibiru/pull/788) - add --overwrite flag to the nibid init call of localnet.sh
Expand Down
17 changes: 9 additions & 8 deletions proto/vpool/v1/vpool.proto
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,14 @@ message Pool {
];
}


// PoolPrices is a simple structure that displays a snapshot of the mark and index
// prices for an asset. Empty strings for the indexPrice or twapMark fields
// indicate that the price is currently unavailable.
message PoolPrices {
// Pair identifier for the two assets. Always in format 'base:quote'
string pair = 9;

// MarkPrice is the instantaneous price of the perp.
// Equivalent to quoteAssetReserve / baseAssetReserve.
string mark_price = 10 [
Expand All @@ -105,16 +112,10 @@ message PoolPrices {
];

// IndexPrice is the price of the "underlying" for the perp
string index_price = 11 [
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the removal of the custom type?

(gogoproto.nullable) = false
];
string index_price = 11;

// TwapMark is the time-weighted average (mark) price.
string twap_mark = 12 [
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Dec",
(gogoproto.nullable) = false
];
string twap_mark = 12;

// SwapInvariant is the product of the reserves, commonly referred to as "k".
string swap_invariant = 13 [
Expand Down
16 changes: 16 additions & 0 deletions x/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,3 +200,19 @@ func (pairs AssetPairs) MarshalJSON() ([]byte, error) {
}
return json.Marshal(assetPairsJSON(pairs))
}

func ToSdkPointer(num interface{}) interface{} {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see where this function is being used in the current PR. Plus I think the name is a little misleading.

switch sdkType := num.(type) {
case sdk.Int:
pointer := new(sdk.Int)
*pointer = num.(sdk.Int)
return pointer
case sdk.Dec:
pointer := new(sdk.Dec)
*pointer = num.(sdk.Dec)
return pointer
default:
errMsg := fmt.Errorf("type passed must be sdk.Int or sdk.Dec, not %s", sdkType)
panic(errMsg)
}
}
3 changes: 1 addition & 2 deletions x/perp/client/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,6 @@ func (s *IntegrationTestSuite) TearDownSuite() {
s.network.Cleanup()
}

// TODO test: Include checks for queryResp.MarginRatioIndex
// https://github.com/NibiruChain/nibiru/issues/809
func (s *IntegrationTestSuite) TestOpenPositionsAndCloseCmd() {
val := s.network.Validators[0]

Expand Down Expand Up @@ -204,6 +202,7 @@ func (s *IntegrationTestSuite) TestOpenPositionsAndCloseCmd() {
s.EqualValues(sdk.MustNewDecFromStr("999999.999999999999999359"), queryResp.PositionNotional)
s.EqualValues(sdk.MustNewDecFromStr("-0.000000000000000641"), queryResp.UnrealizedPnl)
s.EqualValues(sdk.NewDec(1), queryResp.MarginRatioMark)
s.EqualValues(sdk.NewDec(0), queryResp.MarginRatioIndex)

s.T().Log("C. open position with 2x leverage and zero baseAmtLimit")
args = []string{
Expand Down
1 change: 1 addition & 0 deletions x/perp/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ func (q queryServer) QueryTraderPosition(
// The index portion of the query fails silently as not to distrupt all
// position queries when oracles aren't posting prices.
q.Keeper.Logger(ctx).Error(err.Error())
marginRatioIndex = sdk.Dec{}
}

return &types.QueryTraderPositionResponse{
Expand Down
36 changes: 36 additions & 0 deletions x/testutil/mock/mocks.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package mock

import (
"testing"

sdktestsmocks "github.com/cosmos/cosmos-sdk/tests/mocks"
sdk "github.com/cosmos/cosmos-sdk/types"
gomock "github.com/golang/mock/gomock"
)

/* AppendCtxWithMockLogger sets the logger for an input context as a mock logger
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we stick with // comments as they're more idiomatic and common in go

with 'EXPECT' statements. This enables testing on functions logged to the context.
For example,

```go
// This is a passing test example
import (
gomock "github.com/golang/mock/gomock"
sdktestsmocks "github.com/cosmos/cosmos-sdk/tests/mocks"
)
// assume t is a *testing.T variable.
ctx, logger := AppendCtxWithMockLogger(t, ctx)
logger.EXPECT().Debug("debug")
logger.EXPECT().Info("info")
logger.EXPECT().Error("error")

ctx.Logger().Debug("debug")
ctx.Logger().Info("info")
ctx.Logger().Error("error")
```
*/
func AppendCtxWithMockLogger(t *testing.T, ctx sdk.Context) (sdk.Context, *sdktestsmocks.MockLogger) {
ctrl := gomock.NewController(t)
logger := sdktestsmocks.NewMockLogger(ctrl)
return ctx.WithLogger(logger), logger
}
32 changes: 27 additions & 5 deletions x/vpool/keeper/pool_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (k Keeper) getPool(ctx sdk.Context, pair common.AssetPair) (
) {
bz := ctx.KVStore(k.storeKey).Get(types.GetPoolKey(pair))
if bz == nil {
return nil, fmt.Errorf("Could not find vpool for pair %s", pair.String())
return nil, fmt.Errorf("could not find vpool for pair %s", pair.String())
}

var pool types.Pool
Expand Down Expand Up @@ -120,22 +120,44 @@ func (k Keeper) GetAllPools(ctx sdk.Context) []*types.Pool {
}

// GetPoolPrices returns the mark price, twap (mark) price, and index price for a vpool.
func (k Keeper) GetPoolPrices(ctx sdk.Context, pool types.Pool) types.PoolPrices {
// An error is returned if
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forgot to finish documentation?

func (k Keeper) GetPoolPrices(
ctx sdk.Context, pool types.Pool,
) (prices types.PoolPrices, err error) {
// Validation - guarantees no panics in GetUnderlyingPrice or GetCurrentTWAP
if err := pool.Pair.Validate(); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a proof that the current design of AssetPair is not ideal. Ref: #799

return prices, err
}
if !k.ExistsPool(ctx, pool.Pair) {
return prices, types.ErrPairNotSupported.Wrap(pool.Pair.String())
}
if err := pool.ValidateReserves(); err != nil {
return prices, err
}

indexPriceStr := ""
indexPrice, err := k.GetUnderlyingPrice(ctx, pool.Pair)
if err != nil {
// fail gracefully so that vpool queries run even if the oracle price feeds stop
k.Logger(ctx).Error(err.Error())
} else {
indexPriceStr = indexPrice.String()
}
twapMarkStr := ""
twapMark, err := k.GetCurrentTWAP(ctx, pool.Pair)
if err != nil {
// fail gracefully so that vpool queries run even if the TWAP is undefined.
k.Logger(ctx).Error(err.Error())
} else {
twapMarkStr = twapMark.Price.String()
}

return types.PoolPrices{
Pair: pool.Pair.String(),
MarkPrice: pool.QuoteAssetReserve.Quo(pool.BaseAssetReserve),
IndexPrice: indexPrice,
TwapMark: twapMark.Price,
IndexPrice: indexPriceStr,
TwapMark: twapMarkStr,
SwapInvariant: pool.BaseAssetReserve.Mul(pool.QuoteAssetReserve).RoundInt(),
BlockNumber: ctx.BlockHeight(),
}
}, nil
}
Loading