Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

chore!: skip addDenomReverseIndex in bank v3 migration #19266

Merged
merged 5 commits into from
Jan 31, 2024
Merged

chore!: skip addDenomReverseIndex in bank v3 migration #19266

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4135b7b
chore!: skip addDenomReverseIndex in bank v3 migration
MSalopek Jan 26, 2024
0fab7b4
tests: deactivate v3 bank migration tests that are not running
MSalopek Jan 30, 2024
a6fe734
fix!: deactivate DenomOwners query
MSalopek Jan 30, 2024
e39021d
tests: deactivate DenomOwners deterministic test
MSalopek Jan 30, 2024
665f87a
bank: write to denom-to-account reverse index even if index is unused
MSalopek Jan 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 48 additions & 47 deletions tests/integration/bank/keeper/deterministic_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,50 +403,51 @@ func (suite *DeterministicTestSuite) TestGRPCSendEnabled() {
testdata.DeterministicIterations(suite.ctx, suite.Require(), req, suite.queryClient.SendEnabled, 4063, false)
}

func (suite *DeterministicTestSuite) TestGRPCDenomOwners() {
rapid.Check(suite.T(), func(t *rapid.T) {
denom := rapid.StringMatching(denomRegex).Draw(t, "denom")
numAddr := rapid.IntRange(1, 10).Draw(t, "number-address")
for i := 0; i < numAddr; i++ {
addr := testdata.AddressGenerator(t).Draw(t, "address")

coin := sdk.NewCoin(
denom,
sdk.NewInt(rapid.Int64Min(1).Draw(t, "amount")),
)

err := banktestutil.FundAccount(suite.bankKeeper, suite.ctx, addr, sdk.NewCoins(coin))
suite.Require().NoError(err)
}

req := &banktypes.QueryDenomOwnersRequest{
Denom: denom,
Pagination: testdata.PaginationGenerator(t, uint64(numAddr)).Draw(t, "pagination"),
}
testdata.DeterministicIterations(suite.ctx, suite.Require(), req, suite.queryClient.DenomOwners, 0, true)
})

denomOwners := []*banktypes.DenomOwner{
{
Address: "cosmos1qg65a9q6k2sqq7l3ycp428sqqpmqcucgzze299",
Balance: coin1,
},
{
Address: "cosmos1qglnsqgpq48l7qqzgs8qdshr6fh3gqq9ej3qut",
Balance: coin1,
},
}

for i := 0; i < len(denomOwners); i++ {
addr, err := sdk.AccAddressFromBech32(denomOwners[i].Address)
suite.Require().NoError(err)

err = banktestutil.FundAccount(suite.bankKeeper, suite.ctx, addr, sdk.NewCoins(coin1))
suite.Require().NoError(err)
}

req := &banktypes.QueryDenomOwnersRequest{
Denom: coin1.GetDenom(),
}
testdata.DeterministicIterations(suite.ctx, suite.Require(), req, suite.queryClient.DenomOwners, 2525, false)
}
// NOTE: query deactivated, it always returns an error
// func (suite *DeterministicTestSuite) TestGRPCDenomOwners() {
// rapid.Check(suite.T(), func(t *rapid.T) {
// denom := rapid.StringMatching(denomRegex).Draw(t, "denom")
// numAddr := rapid.IntRange(1, 10).Draw(t, "number-address")
// for i := 0; i < numAddr; i++ {
// addr := testdata.AddressGenerator(t).Draw(t, "address")

// coin := sdk.NewCoin(
// denom,
// sdk.NewInt(rapid.Int64Min(1).Draw(t, "amount")),
// )

// err := banktestutil.FundAccount(suite.bankKeeper, suite.ctx, addr, sdk.NewCoins(coin))
// suite.Require().NoError(err)
// }

// req := &banktypes.QueryDenomOwnersRequest{
// Denom: denom,
// Pagination: testdata.PaginationGenerator(t, uint64(numAddr)).Draw(t, "pagination"),
// }
// testdata.DeterministicIterations(suite.ctx, suite.Require(), req, suite.queryClient.DenomOwners, 0, true)
// })

// denomOwners := []*banktypes.DenomOwner{
// {
// Address: "cosmos1qg65a9q6k2sqq7l3ycp428sqqpmqcucgzze299",
// Balance: coin1,
// },
// {
// Address: "cosmos1qglnsqgpq48l7qqzgs8qdshr6fh3gqq9ej3qut",
// Balance: coin1,
// },
// }

// for i := 0; i < len(denomOwners); i++ {
// addr, err := sdk.AccAddressFromBech32(denomOwners[i].Address)
// suite.Require().NoError(err)

// err = banktestutil.FundAccount(suite.bankKeeper, suite.ctx, addr, sdk.NewCoins(coin1))
// suite.Require().NoError(err)
// }

// req := &banktypes.QueryDenomOwnersRequest{
// Denom: coin1.GetDenom(),
// }
// testdata.DeterministicIterations(suite.ctx, suite.Require(), req, suite.queryClient.DenomOwners, 2525, false)
// }
36 changes: 1 addition & 35 deletions x/bank/keeper/grpc_query.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,41 +224,7 @@ func (k BaseKeeper) DenomOwners(
return nil, status.Errorf(codes.InvalidArgument, "empty request")
}

if err := sdk.ValidateDenom(req.Denom); err != nil {
return nil, status.Error(codes.InvalidArgument, err.Error())
}

ctx := sdk.UnwrapSDKContext(goCtx)
denomPrefixStore := k.getDenomAddressPrefixStore(ctx, req.Denom)

var denomOwners []*types.DenomOwner
pageRes, err := query.FilteredPaginate(
denomPrefixStore,
req.Pagination,
func(key []byte, _ []byte, accumulate bool) (bool, error) {
if accumulate {
address, _, err := types.AddressAndDenomFromBalancesStore(key)
if err != nil {
return false, err
}

denomOwners = append(
denomOwners,
&types.DenomOwner{
Address: address.String(),
Balance: k.GetBalance(ctx, address, req.Denom),
},
)
}

return true, nil
},
)
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}

return &types.QueryDenomOwnersResponse{DenomOwners: denomOwners, Pagination: pageRes}, nil
return nil, status.Error(codes.Unimplemented, "not implemented on CosmosHub. Use an indexer.")
}

func (k BaseKeeper) SendEnabled(goCtx context.Context, req *types.QuerySendEnabledRequest) (*types.QuerySendEnabledResponse, error) {
Expand Down
23 changes: 6 additions & 17 deletions x/bank/keeper/grpc_query_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,7 @@ func (suite *KeeperTestSuite) QueryDenomMetadataRequest() {
}
}

// NOTE: DenomOwners is not implemented (returns an error) on Cosmos Hub
func (suite *KeeperTestSuite) TestGRPCDenomOwners() {
ctx := suite.ctx

Expand Down Expand Up @@ -456,7 +457,7 @@ func (suite *KeeperTestSuite) TestGRPCDenomOwners() {
req: &types.QueryDenomOwnersRequest{
Denom: "foo",
},
expPass: true,
expPass: false,
numAddrs: 0,
hasNext: false,
total: 0,
Expand All @@ -469,7 +470,7 @@ func (suite *KeeperTestSuite) TestGRPCDenomOwners() {
CountTotal: true,
},
},
expPass: true,
expPass: false,
numAddrs: 6,
hasNext: true,
total: 10,
Expand All @@ -483,7 +484,7 @@ func (suite *KeeperTestSuite) TestGRPCDenomOwners() {
CountTotal: true,
},
},
expPass: true,
expPass: false,
numAddrs: 4,
hasNext: false,
total: 10,
Expand All @@ -493,20 +494,8 @@ func (suite *KeeperTestSuite) TestGRPCDenomOwners() {
for name, tc := range testCases {
suite.Run(name, func() {
resp, err := suite.queryClient.DenomOwners(gocontext.Background(), tc.req)
if tc.expPass {
suite.NoError(err)
suite.NotNil(resp)
suite.Len(resp.DenomOwners, tc.numAddrs)
suite.Equal(tc.total, resp.Pagination.Total)

if tc.hasNext {
suite.NotNil(resp.Pagination.NextKey)
} else {
suite.Nil(resp.Pagination.NextKey)
}
} else {
suite.Require().Error(err)
}
suite.Require().Error(err)
suite.Require().Nil(resp)
})
}

Expand Down
3 changes: 3 additions & 0 deletions x/bank/keeper/send.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,9 @@ func (k BaseSendKeeper) setBalance(ctx sdk.Context, addr sdk.AccAddress, balance

accountStore.Set([]byte(balance.Denom), amount)

// NOTE: parts of x/bank v3 migration were skipped
// but we will keep writing to the denom to account reverse index

// Store a reverse index from denomination to account address with a
// sentinel value.
denomAddrKey := address.MustLengthPrefix(addr)
Expand Down
83 changes: 43 additions & 40 deletions x/bank/migrations/v3/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import (
"github.com/cosmos/cosmos-sdk/store/prefix"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/address"

// "github.com/cosmos/cosmos-sdk/types/address"
v2 "github.com/cosmos/cosmos-sdk/x/bank/migrations/v2"
"github.com/cosmos/cosmos-sdk/x/bank/types"
)
Expand All @@ -19,59 +20,61 @@ import (
// - Remove duplicate denom from denom metadata store key.
func MigrateStore(ctx sdk.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec) error {
store := ctx.KVStore(storeKey)
err := addDenomReverseIndex(store, cdc, ctx.Logger())
if err != nil {
return err
}
// NOTE: deactivating this migration for now, as it is not required and executing it is expensive
// err := addDenomReverseIndex(store, cdc, ctx.Logger())
// if err != nil {
// return err
// }

return migrateDenomMetadata(store, ctx.Logger())
}

func addDenomReverseIndex(store sdk.KVStore, cdc codec.BinaryCodec, logger log.Logger) error {
oldBalancesStore := prefix.NewStore(store, v2.BalancesPrefix)
// NOTE: deactivating this migration for now, as it is not required and executing it is expensive
// func addDenomReverseIndex(store sdk.KVStore, cdc codec.BinaryCodec, logger log.Logger) error {
// oldBalancesStore := prefix.NewStore(store, v2.BalancesPrefix)

oldBalancesIter := oldBalancesStore.Iterator(nil, nil)
defer sdk.LogDeferred(logger, func() error { return oldBalancesIter.Close() })
// oldBalancesIter := oldBalancesStore.Iterator(nil, nil)
// defer sdk.LogDeferred(logger, func() error { return oldBalancesIter.Close() })

denomPrefixStores := make(map[string]prefix.Store) // memoize prefix stores
// denomPrefixStores := make(map[string]prefix.Store) // memoize prefix stores

for ; oldBalancesIter.Valid(); oldBalancesIter.Next() {
var balance sdk.Coin
if err := cdc.Unmarshal(oldBalancesIter.Value(), &balance); err != nil {
return err
}
// for ; oldBalancesIter.Valid(); oldBalancesIter.Next() {
// var balance sdk.Coin
// if err := cdc.Unmarshal(oldBalancesIter.Value(), &balance); err != nil {
// return err
// }

addr, err := v2.AddressFromBalancesStore(oldBalancesIter.Key())
if err != nil {
return err
}
// addr, err := v2.AddressFromBalancesStore(oldBalancesIter.Key())
// if err != nil {
// return err
// }

var coin sdk.DecCoin
if err := cdc.Unmarshal(oldBalancesIter.Value(), &coin); err != nil {
return err
}
// var coin sdk.DecCoin
// if err := cdc.Unmarshal(oldBalancesIter.Value(), &coin); err != nil {
// return err
// }

bz, err := coin.Amount.Marshal()
if err != nil {
return err
}
// bz, err := coin.Amount.Marshal()
// if err != nil {
// return err
// }

newStore := prefix.NewStore(store, types.CreateAccountBalancesPrefix(addr))
newStore.Set([]byte(coin.Denom), bz)
// newStore := prefix.NewStore(store, types.CreateAccountBalancesPrefix(addr))
// newStore.Set([]byte(coin.Denom), bz)

denomPrefixStore, ok := denomPrefixStores[balance.Denom]
if !ok {
denomPrefixStore = prefix.NewStore(store, CreateDenomAddressPrefix(balance.Denom))
denomPrefixStores[balance.Denom] = denomPrefixStore
}
// denomPrefixStore, ok := denomPrefixStores[balance.Denom]
// if !ok {
// denomPrefixStore = prefix.NewStore(store, CreateDenomAddressPrefix(balance.Denom))
// denomPrefixStores[balance.Denom] = denomPrefixStore
// }

// Store a reverse index from denomination to account address with a
// sentinel value.
denomPrefixStore.Set(address.MustLengthPrefix(addr), []byte{0})
}
// // Store a reverse index from denomination to account address with a
// // sentinel value.
// denomPrefixStore.Set(address.MustLengthPrefix(addr), []byte{0})
// }

return nil
}
// return nil
// }

func migrateDenomMetadata(store sdk.KVStore, logger log.Logger) error {
oldDenomMetaDataStore := prefix.NewStore(store, v2.DenomMetadataPrefix)
Expand Down
Loading
Loading