Skip to content

Commit

Permalink
refactor(common): Remove unused AssetPair methods (#902)
Browse files Browse the repository at this point in the history
* refactor: rename pair and denom constants

* refactor(common): remove unused AssetPair methods

* chore: update changelog
  • Loading branch information
NibiruHeisenberg authored Sep 14, 2022
1 parent 2701529 commit 6e9a0b1
Show file tree
Hide file tree
Showing 68 changed files with 1,030 additions and 1,159 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* [#876](https://github.com/NibiruChain/nibiru/pull/876) - chore(deps): bump github.com/spf13/viper from 1.12.0 to 1.13.0
* [#889](https://github.com/NibiruChain/nibiru/pull/889) - feat: decouple keeper from servers in pricefeed module
* [#886](https://github.com/NibiruChain/nibiru/pull/886) - feat: decouple keeper from servers in perp module
* [#902](https://github.com/NibiruChain/nibiru/pull/902) - refactor(common): improve usability of `common.AssetPair`

### Features

Expand Down
2 changes: 1 addition & 1 deletion simapp/testapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func NewTestGenesisState(codec codec.Codec, inGenState GenesisState,
var govGenState govtypes.GenesisState
codec.MustUnmarshalJSON(testGenState[govtypes.ModuleName], &govGenState)
govGenState.VotingParams.VotingPeriod = time.Second * 20
govGenState.DepositParams.MinDeposit = sdk.NewCoins(sdk.NewInt64Coin(common.DenomGov, 1_000_000)) // min deposit of 1 NIBI
govGenState.DepositParams.MinDeposit = sdk.NewCoins(sdk.NewInt64Coin(common.DenomNIBI, 1_000_000)) // min deposit of 1 NIBI
testGenState[govtypes.ModuleName] = codec.MustMarshalJSON(&govGenState)

// pricefeed genesis state
Expand Down
4 changes: 2 additions & 2 deletions simapp/testapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ func (s *TestappSuite) TestPricefeedGenesis_PostedPrices() {
}
currentPrices = nibiruApp.PricefeedKeeper.GetCurrentPrices(ctx)
s.Assert().Len(currentPrices, 2)
s.Assert().Equal(common.PairGovStable.String(), currentPrices[0].PairID)
s.Assert().Equal(common.PairCollStable.String(), currentPrices[1].PairID)
s.Assert().Equal(common.Pair_NIBI_NUSD.String(), currentPrices[0].PairID)
s.Assert().Equal(common.Pair_USDC_NUSD.String(), currentPrices[1].PairID)
}

func TestTestappSuite(t *testing.T) {
Expand Down
74 changes: 17 additions & 57 deletions x/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,18 @@ package common
import (
"encoding/json"
"fmt"
"sort"
"strings"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)

const (
DenomGov = "unibi"
DenomColl = "uusdc"
DenomStable = "unusd"
DenomBTC = "ubtc"
DenomETH = "ueth"
DenomNIBI = "unibi"
DenomUSDC = "uusdc"
DenomNUSD = "unusd"
DenomBTC = "ubtc"
DenomETH = "ueth"

ModuleName = "common"

Expand All @@ -25,10 +24,10 @@ const (
)

var (
PairGovStable = AssetPair{Token0: DenomGov, Token1: DenomStable}
PairCollStable = AssetPair{Token0: DenomColl, Token1: DenomStable}
PairBTCStable = AssetPair{Token0: DenomBTC, Token1: DenomStable}
PairETHStable = AssetPair{Token0: DenomETH, Token1: DenomStable}
Pair_NIBI_NUSD = AssetPair{Token0: DenomNIBI, Token1: DenomNUSD}
Pair_USDC_NUSD = AssetPair{Token0: DenomUSDC, Token1: DenomNUSD}
Pair_BTC_NUSD = AssetPair{Token0: DenomBTC, Token1: DenomNUSD}
Pair_ETH_NUSD = AssetPair{Token0: DenomETH, Token1: DenomNUSD}

ErrInvalidTokenPair = sdkerrors.Register(ModuleName, 1, "invalid token pair")
)
Expand Down Expand Up @@ -71,24 +70,15 @@ func MustNewAssetPair(pair string) AssetPair {
return assetPair
}

// SortedName is the string representation of the pair with sorted assets.
func (pair AssetPair) SortedName() string {
return SortedPairNameFromDenoms([]string{pair.Token0, pair.Token1})
}

/*
String returns the string representation of the asset pair.
String returns the string representation of the asset pair.
Note that this differs from the output of the proto-generated 'String' method.
*/
func (pair AssetPair) String() string {
return fmt.Sprintf("%s%s%s", pair.Token0, PairSeparator, pair.Token1)
}

func (pair AssetPair) IsSortedOrder() bool {
return pair.SortedName() == pair.String()
}

func (pair AssetPair) Inverse() AssetPair {
return AssetPair{pair.Token1, pair.Token0}
}
Expand All @@ -101,28 +91,6 @@ func (pair AssetPair) QuoteDenom() string {
return pair.Token1
}

func DenomsFromPoolName(pool string) (denoms []string) {
return strings.Split(pool, ":")
}

// SortedPairNameFromDenoms returns a sorted string representing a pool of assets
func SortedPairNameFromDenoms(denoms []string) string {
sort.Strings(denoms) // alphabetically sort in-place
return PairNameFromDenoms(denoms)
}

// PairNameFromDenoms returns a string representing a pool of assets in the
// exact order the denoms were given as args
func PairNameFromDenoms(denoms []string) string {
poolName := denoms[0]
for idx, denom := range denoms {
if idx != 0 {
poolName += fmt.Sprintf("%s%s", PairSeparator, denom)
}
}
return poolName
}

// Validate performs a basic validation of the market params
func (pair AssetPair) Validate() error {
if err := sdk.ValidateDenom(pair.Token1); err != nil {
Expand Down Expand Up @@ -150,9 +118,13 @@ func NewAssetPairs(pairStrings ...string) (pairs AssetPairs) {
}

// Contains checks if a token pair is contained within 'Pairs'
func (pairs AssetPairs) Contains(pair AssetPair) bool {
isContained, _ := pairs.ContainsAtIndex(pair)
return isContained
func (haystack AssetPairs) Contains(needle AssetPair) bool {
for _, p := range haystack {
if p.Equal(needle) {
return true
}
}
return false
}

func (pairs AssetPairs) Strings() []string {
Expand All @@ -178,18 +150,6 @@ func (pairs AssetPairs) Validate() error {
return nil
}

// ContainsAtIndex checks if a token pair is contained within 'Pairs' and
// a boolean for this condition alongside the corresponding index of 'pair' in
// the slice of pairs.
func (pairs AssetPairs) ContainsAtIndex(pair AssetPair) (bool, int) {
for idx, element := range pairs {
if (element.Token0 == pair.Token0) && (element.Token1 == pair.Token1) {
return true, idx
}
}
return false, -1
}

type assetPairsJSON AssetPairs

// MarshalJSON implements a custom JSON marshaller for the AssetPairs type to allow
Expand Down
102 changes: 5 additions & 97 deletions x/common/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,85 +4,11 @@ import (
"fmt"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/NibiruChain/nibiru/x/common"
)

func TestPairNameFromDenoms(t *testing.T) {
testCases := []struct {
name string
denoms []string
poolName string
}{
{
name: "ATOM:OSMO in correct order",
denoms: []string{"atom", "osmo"},
poolName: "atom:osmo",
},
{
name: "ATOM:OSMO in wrong order",
denoms: []string{"osmo", "atom"},
poolName: "atom:osmo",
},
{
name: "X:Y:Z in correct order",
denoms: []string{"x", "y", "z"},
poolName: "x:y:z",
},
{
name: "X:Y:Z in wrong order",
denoms: []string{"z", "x", "y"},
poolName: "x:y:z",
},
}

for _, testCase := range testCases {
tc := testCase
t.Run(tc.name, func(t *testing.T) {
outPoolName := common.SortedPairNameFromDenoms(tc.denoms)
require.Equal(t, tc.poolName, outPoolName)
})
}
}

func TestAssetPair_InverseAndSort(t *testing.T) {
testCases := []struct {
name string
pair common.AssetPair
proper bool
}{
{
name: "proper and improper order pairs are inverses-1",
pair: common.AssetPair{Token0: "atom", Token1: "osmo"},
proper: true,
},
{
name: "proper and improper order pairs are inverses-2",
pair: common.AssetPair{Token0: "osmo", Token1: "atom"},
proper: false,
},
}

for _, testCase := range testCases {
tc := testCase
t.Run(tc.name, func(t *testing.T) {
if tc.proper {
require.Truef(t, tc.pair.IsSortedOrder(),
"pair: '%v' name: '%v'", tc.pair.String(), tc.pair.SortedName())
require.EqualValues(t, tc.pair.SortedName(), tc.pair.String())
} else {
require.Truef(t, tc.pair.Inverse().IsSortedOrder(),
"pair: '%v' name: '%v'", tc.pair.String(), tc.pair.SortedName())
require.EqualValues(t, tc.pair.SortedName(), tc.pair.Inverse().String())
}

require.True(t, true)
})
}
}

func TestNewAssetPair_Constructor(t *testing.T) {
tests := []struct {
name string
Expand All @@ -91,23 +17,23 @@ func TestNewAssetPair_Constructor(t *testing.T) {
}{
{
"only one token",
common.DenomGov,
common.DenomNIBI,
common.ErrInvalidTokenPair,
},
{
"more than 2 tokens",
fmt.Sprintf("%s%s%s%s%s", common.DenomGov, common.PairSeparator, common.DenomStable,
common.PairSeparator, common.DenomColl),
fmt.Sprintf("%s%s%s%s%s", common.DenomNIBI, common.PairSeparator, common.DenomNUSD,
common.PairSeparator, common.DenomUSDC),
common.ErrInvalidTokenPair,
},
{
"different separator",
fmt.Sprintf("%s%s%s", common.DenomGov, "%", common.DenomStable),
fmt.Sprintf("%s%s%s", common.DenomNIBI, "%", common.DenomNUSD),
common.ErrInvalidTokenPair,
},
{
"correct pair",
fmt.Sprintf("%s%s%s", common.DenomGov, common.PairSeparator, common.DenomStable),
fmt.Sprintf("%s%s%s", common.DenomNIBI, common.PairSeparator, common.DenomNUSD),
nil,
},
{
Expand Down Expand Up @@ -181,21 +107,3 @@ func TestAssetPair_Marshaling(t *testing.T) {
})
}
}

func TestAssetPairs_Contains(t *testing.T) {
pairs := common.AssetPairs{
common.PairBTCStable, common.PairETHStable,
}

pair := common.PairGovStable
isContained, atIdx := pairs.ContainsAtIndex(pair)
assert.False(t, isContained)
assert.Equal(t, -1, atIdx)
assert.False(t, pairs.Contains(pair))

pair = pairs[0]
isContained, atIdx = pairs.ContainsAtIndex(pair)
assert.True(t, isContained)
assert.Equal(t, 0, atIdx)
assert.True(t, pairs.Contains(pair))
}
8 changes: 4 additions & 4 deletions x/dex/client/testutil/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ import (

func TestIntegrationTestSuite(t *testing.T) {
coinsFromGenesis := []string{
common.DenomGov,
common.DenomStable,
common.DenomColl,
common.DenomNIBI,
common.DenomNUSD,
common.DenomUSDC,
"coin-1",
"coin-2",
"coin-3",
Expand All @@ -34,7 +34,7 @@ func TestIntegrationTestSuite(t *testing.T) {

cfg := testutilcli.BuildNetworkConfig(genesisState)
cfg.StartingTokens = sdk.NewCoins(
sdk.NewInt64Coin(common.DenomGov, 2e12), // for pool creation fee and more for tx fees
sdk.NewInt64Coin(common.DenomNIBI, 2e12), // for pool creation fee and more for tx fees
)

for _, coin := range coinsFromGenesis {
Expand Down
2 changes: 1 addition & 1 deletion x/dex/client/testutil/test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
var commonArgs = []string{
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(common.DenomGov, sdk.NewInt(10))).String()),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(common.DenomNIBI, sdk.NewInt(10))).String()),
}

// ExecMsgCreatePool broadcast a pool creation message.
Expand Down
6 changes: 3 additions & 3 deletions x/dex/keeper/balances_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ func TestCheckBalances(t *testing.T) {
name: "has enough funds",
userInitialFunds: sdk.NewCoins(
sdk.NewInt64Coin("unibi", 100),
sdk.NewInt64Coin(common.DenomStable, 100),
sdk.NewInt64Coin(common.DenomNUSD, 100),
),
coinsToSpend: sdk.NewCoins(
sdk.NewInt64Coin("unibi", 100),
sdk.NewInt64Coin(common.DenomStable, 100),
sdk.NewInt64Coin(common.DenomNUSD, 100),
),
expectedError: nil,
},
Expand All @@ -44,7 +44,7 @@ func TestCheckBalances(t *testing.T) {
),
coinsToSpend: sdk.NewCoins(
sdk.NewInt64Coin("unibi", 100),
sdk.NewInt64Coin(common.DenomStable, 100),
sdk.NewInt64Coin(common.DenomNUSD, 100),
),
expectedError: sdkerrors.ErrInsufficientFunds,
},
Expand Down
Loading

0 comments on commit 6e9a0b1

Please sign in to comment.