Skip to content

Commit

Permalink
refactor: audit x/bank package changes in 0.47 release (cosmos#14076)
Browse files Browse the repository at this point in the history
  • Loading branch information
@alexanderbez
alexanderbez committed Dec 9, 2022
1 parent 7ded321 commit 18b0fa0
Show file tree
Hide file tree
Showing 9 changed files with 148 additions and 173 deletions.
9 changes: 7 additions & 2 deletions x/bank/keeper/grpc_query.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"context"

"cosmossdk.io/math"

gogotypes "github.com/cosmos/gogoproto/types"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

Expand Down Expand Up @@ -262,12 +262,16 @@ func (k BaseKeeper) SendEnabled(goCtx context.Context, req *types.QuerySendEnabl
} else {
store := k.getSendEnabledPrefixStore(ctx)
var err error

resp.Pagination, err = query.FilteredPaginate(
store,
req.Pagination,
func(key []byte, value []byte, accumulate bool) (bool, error) {
if accumulate {
resp.SendEnabled = append(resp.SendEnabled, types.NewSendEnabled(string(key), types.IsTrueB(value)))
var enabled gogotypes.BoolValue
k.cdc.MustUnmarshal(value, &enabled)

resp.SendEnabled = append(resp.SendEnabled, types.NewSendEnabled(string(key), enabled.Value))
}
return true, nil
},
Expand All @@ -276,5 +280,6 @@ func (k BaseKeeper) SendEnabled(goCtx context.Context, req *types.QuerySendEnabl
return nil, status.Error(codes.Internal, err.Error())
}
}

return resp, nil
}
80 changes: 50 additions & 30 deletions x/bank/keeper/keeper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ import (

"cosmossdk.io/math"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"

abci "github.com/tendermint/tendermint/abci/types"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
tmtime "github.com/tendermint/tendermint/types/time"
Expand All @@ -20,13 +18,11 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil"
"github.com/cosmos/cosmos-sdk/types/query"

authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
vesting "github.com/cosmos/cosmos-sdk/x/auth/vesting/types"
"github.com/cosmos/cosmos-sdk/x/bank/exported"
"github.com/cosmos/cosmos-sdk/x/bank/keeper"
banktestutil "github.com/cosmos/cosmos-sdk/x/bank/testutil"

banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
minttypes "github.com/cosmos/cosmos-sdk/x/mint/types"
Expand Down Expand Up @@ -84,6 +80,10 @@ type KeeperTestSuite struct {
encCfg moduletestutil.TestEncodingConfig
}

func TestKeeperTestSuite(t *testing.T) {
suite.Run(t, new(KeeperTestSuite))
}

func (suite *KeeperTestSuite) SetupTest() {
key := sdk.NewKVStoreKey(banktypes.StoreKey)
testCtx := testutil.DefaultContextWithDB(suite.T(), key, sdk.NewTransientStoreKey("transient_test"))
Expand Down Expand Up @@ -216,7 +216,7 @@ func (suite *KeeperTestSuite) TestGetAuthority() {
suite.T().Run(name, func(t *testing.T) {
kpr := NewKeeperWithAuthority(expected)
actual := kpr.GetAuthority()
assert.Equal(t, expected, actual)
suite.Require().Equal(expected, actual)
})
}
}
Expand Down Expand Up @@ -1367,21 +1367,24 @@ func (suite *KeeperTestSuite) TestIsSendEnabledDenom() {
for _, def := range []bool{true, false} {
params := banktypes.Params{DefaultSendEnabled: def}
require.NoError(bankKeeper.SetParams(ctx, params))

for _, tc := range tests {
suite.T().Run(fmt.Sprintf("%s default %t", tc.denom, def), func(t *testing.T) {
actual := suite.bankKeeper.IsSendEnabledDenom(suite.ctx, tc.denom)
exp := tc.exp
if tc.expDef {
exp = def
}
assert.Equal(t, exp, actual)

require.Equal(exp, actual)
})
}
}
}

func (suite *KeeperTestSuite) TestGetSendEnabledEntry() {
ctx, bankKeeper := suite.ctx, suite.bankKeeper
require := suite.Require()

bankKeeper.SetAllSendEnabled(ctx, []*banktypes.SendEnabled{
{Denom: "gettruecoin", Enabled: true},
Expand Down Expand Up @@ -1413,8 +1416,8 @@ func (suite *KeeperTestSuite) TestGetSendEnabledEntry() {
for _, tc := range tests {
suite.T().Run(tc.denom, func(t *testing.T) {
actualSE, actualF := bankKeeper.GetSendEnabledEntry(ctx, tc.denom)
assert.Equal(t, tc.expF, actualF, "found")
assert.Equal(t, tc.expSE, actualSE, "SendEnabled")
require.Equal(tc.expF, actualF, "found")
require.Equal(tc.expSE, actualSE, "SendEnabled")
})
}
}
Expand Down Expand Up @@ -1473,11 +1476,12 @@ func (suite *KeeperTestSuite) TestSetSendEnabled() {
for _, def := range []bool{true, false} {
params := banktypes.Params{DefaultSendEnabled: def}
require.NoError(bankKeeper.SetParams(ctx, params))

for _, tc := range tests {
suite.T().Run(fmt.Sprintf("%s default %t", tc.name, def), func(t *testing.T) {
bankKeeper.SetSendEnabled(ctx, tc.denom, tc.value)
actual := bankKeeper.IsSendEnabledDenom(ctx, tc.denom)
assert.Equal(t, tc.value, actual)
require.Equal(tc.value, actual)
})
}
}
Expand Down Expand Up @@ -1544,12 +1548,14 @@ func (suite *KeeperTestSuite) TestSetAllSendEnabled() {
for _, def := range []bool{true, false} {
params := banktypes.Params{DefaultSendEnabled: def}
require.NoError(bankKeeper.SetParams(ctx, params))

for _, tc := range tests {
suite.T().Run(fmt.Sprintf("%s default %t", tc.name, def), func(t *testing.T) {
bankKeeper.SetAllSendEnabled(ctx, tc.sendEnableds)

for _, se := range tc.sendEnableds {
actual := bankKeeper.IsSendEnabledDenom(ctx, se.Denom)
assert.Equal(t, se.Enabled, actual, se.Denom)
require.Equal(se.Enabled, actual, se.Denom)
}
})
}
Expand Down Expand Up @@ -1583,7 +1589,8 @@ func (suite *KeeperTestSuite) TestIterateSendEnabledEntries() {
count++
return false
})
assert.Equal(t, 0, count)

require.Equal(0, count)
})

alpha := strings.Split("abcdefghijklmnopqrstuvwxyz", "")
Expand All @@ -1598,6 +1605,7 @@ func (suite *KeeperTestSuite) TestIterateSendEnabledEntries() {
for _, def := range []bool{true, false} {
params := banktypes.Params{DefaultSendEnabled: def}
require.NoError(bankKeeper.SetParams(ctx, params))

var seen []string
suite.T().Run(fmt.Sprintf("all denoms have expected values default %t", def), func(t *testing.T) {
bankKeeper.IterateSendEnabledEntries(ctx, func(denom string, sendEnabled bool) (stop bool) {
Expand All @@ -1606,12 +1614,14 @@ func (suite *KeeperTestSuite) TestIterateSendEnabledEntries() {
if strings.HasSuffix(denom, "false") {
exp = false
}
assert.Equal(t, exp, sendEnabled, denom)

require.Equal(exp, sendEnabled, denom)
return false
})
})

suite.T().Run(fmt.Sprintf("all denoms were seen default %t", def), func(t *testing.T) {
assert.ElementsMatch(t, denoms, seen)
require.ElementsMatch(denoms, seen)
})
}

Expand All @@ -1623,7 +1633,8 @@ func (suite *KeeperTestSuite) TestIterateSendEnabledEntries() {
count++
return false
})
assert.Equal(t, 0, count)

require.Equal(0, count)
})
}

Expand All @@ -1633,7 +1644,7 @@ func (suite *KeeperTestSuite) TestGetAllSendEnabledEntries() {

suite.T().Run("no entries", func(t *testing.T) {
actual := bankKeeper.GetAllSendEnabledEntries(ctx)
assert.Len(t, actual, 0)
require.Len(actual, 0)
})

alpha := strings.Split("abcdefghijklmnopqrstuvwxyz", "")
Expand All @@ -1648,6 +1659,7 @@ func (suite *KeeperTestSuite) TestGetAllSendEnabledEntries() {
for _, def := range []bool{true, false} {
params := banktypes.Params{DefaultSendEnabled: def}
require.NoError(bankKeeper.SetParams(ctx, params))

var seen []string
suite.T().Run(fmt.Sprintf("all denoms have expected values default %t", def), func(t *testing.T) {
actual := bankKeeper.GetAllSendEnabledEntries(ctx)
Expand All @@ -1657,11 +1669,13 @@ func (suite *KeeperTestSuite) TestGetAllSendEnabledEntries() {
if strings.HasSuffix(se.Denom, "false") {
exp = false
}
assert.Equal(t, exp, se.Enabled, se.Denom)

require.Equal(exp, se.Enabled, se.Denom)
}
})

suite.T().Run(fmt.Sprintf("all denoms were seen default %t", def), func(t *testing.T) {
assert.ElementsMatch(t, denoms, seen)
require.ElementsMatch(denoms, seen)
})
}

Expand All @@ -1671,7 +1685,7 @@ func (suite *KeeperTestSuite) TestGetAllSendEnabledEntries() {

suite.T().Run("no entries again after deleting all of them", func(t *testing.T) {
actual := bankKeeper.GetAllSendEnabledEntries(ctx)
assert.Len(t, actual, 0)
require.Len(actual, 0)
})
}

Expand All @@ -1690,12 +1704,15 @@ func (suite *KeeperTestSuite) TestMigrator_Migrate3to4() {
for _, def := range []bool{true, false} {
params := banktypes.Params{DefaultSendEnabled: def}
require.NoError(bankKeeper.SetParams(ctx, params))

suite.T().Run(fmt.Sprintf("default %t does not change", def), func(t *testing.T) {
legacySubspace := func(ps banktypes.Params) mockSubspace {
return mockSubspace{ps: ps}
}(banktypes.NewParams(def))

migrator := keeper.NewMigrator(bankKeeper, legacySubspace)
require.NoError(migrator.Migrate3to4(ctx))

actual := bankKeeper.GetParams(ctx)
require.Equal(params.DefaultSendEnabled, actual.DefaultSendEnabled)
})
Expand All @@ -1708,18 +1725,24 @@ func (suite *KeeperTestSuite) TestMigrator_Migrate3to4() {
{Denom: fmt.Sprintf("falsecoin%t", def), Enabled: false},
},
}

require.NoError(bankKeeper.SetParams(ctx, params))

suite.T().Run(fmt.Sprintf("default %t send enabled info moved to store", def), func(t *testing.T) {
legacySubspace := func(ps banktypes.Params) mockSubspace {
return mockSubspace{ps: ps}
}(banktypes.NewParams(def))

migrator := keeper.NewMigrator(bankKeeper, legacySubspace)
require.NoError(migrator.Migrate3to4(ctx))

newParams := bankKeeper.GetParams(ctx)
assert.Len(t, newParams.SendEnabled, 0)
require.Len(newParams.SendEnabled, 0)
require.Equal(def, newParams.DefaultSendEnabled)

for _, se := range params.SendEnabled {
actual := bankKeeper.IsSendEnabledDenom(ctx, se.Denom)
assert.Equal(t, se.Enabled, actual, se.Denom)
require.Equal(se.Enabled, actual, se.Denom)
}
})
}
Expand All @@ -1728,6 +1751,7 @@ func (suite *KeeperTestSuite) TestMigrator_Migrate3to4() {
func (suite *KeeperTestSuite) TestSetParams() {
ctx, bankKeeper := suite.ctx, suite.bankKeeper
require := suite.Require()

params := banktypes.NewParams(true)
params.SendEnabled = []*banktypes.SendEnabled{
{Denom: "paramscointrue", Enabled: true},
Expand All @@ -1737,21 +1761,17 @@ func (suite *KeeperTestSuite) TestSetParams() {

suite.Run("stored params are as expected", func() {
actual := bankKeeper.GetParams(ctx)
suite.Assert().True(actual.DefaultSendEnabled, "DefaultSendEnabled")
suite.Assert().Len(actual.SendEnabled, 0, "SendEnabled")
require.True(actual.DefaultSendEnabled, "DefaultSendEnabled")
require.Len(actual.SendEnabled, 0, "SendEnabled")
})

suite.Run("send enabled params converted to store", func() {
actual := bankKeeper.GetAllSendEnabledEntries(ctx)
if suite.Assert().Len(actual, 2) {
suite.Equal("paramscoinfalse", actual[0].Denom, "actual[0].Denom")
suite.False(actual[0].Enabled, "actual[0].Enabled")
suite.Equal("paramscointrue", actual[1].Denom, "actual[1].Denom")
suite.True(actual[1].Enabled, "actual[1].Enabled")
require.Equal("paramscoinfalse", actual[0].Denom, "actual[0].Denom")
require.False(actual[0].Enabled, "actual[0].Enabled")
require.Equal("paramscointrue", actual[1].Denom, "actual[1].Denom")
require.True(actual[1].Enabled, "actual[1].Enabled")
}
})
}

func TestKeeperTestSuite(t *testing.T) {
suite.Run(t, new(KeeperTestSuite))
}
Loading

0 comments on commit 18b0fa0

Please sign in to comment.