diff --git a/CHANGELOG.md b/CHANGELOG.md index 2afdf3b6c6d6..be5c97c17b6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +### Bug Fixes + +* (x/bank) [#16841](https://github.com/cosmos/cosmos-sdk/pull/16841) Correctly process legacy `DenomAddressIndex` values. + ### API Breaking Changes * (types/math) [#16040](https://github.com/cosmos/cosmos-sdk/pull/16798) Remove aliases in `types/math.go` (part 2). diff --git a/x/bank/keeper/collections_test.go b/x/bank/keeper/collections_test.go new file mode 100644 index 000000000000..e1af343cce56 --- /dev/null +++ b/x/bank/keeper/collections_test.go @@ -0,0 +1,85 @@ +package keeper_test + +import ( + "testing" + + cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" + cmttime "github.com/cometbft/cometbft/types/time" + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/require" + + "cosmossdk.io/collections" + "cosmossdk.io/log" + "cosmossdk.io/math" + storetypes "cosmossdk.io/store/types" + + "github.com/cosmos/cosmos-sdk/codec/address" + "github.com/cosmos/cosmos-sdk/runtime" + "github.com/cosmos/cosmos-sdk/testutil" + sdk "github.com/cosmos/cosmos-sdk/types" + moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + "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" +) + +func TestBankStateCompatibility(t *testing.T) { + key := storetypes.NewKVStoreKey(banktypes.StoreKey) + testCtx := testutil.DefaultContextWithDB(t, key, storetypes.NewTransientStoreKey("transient_test")) + ctx := testCtx.Ctx.WithBlockHeader(cmtproto.Header{Time: cmttime.Now()}) + encCfg := moduletestutil.MakeTestEncodingConfig() + + storeService := runtime.NewKVStoreService(key) + + // gomock initializations + ctrl := gomock.NewController(t) + authKeeper := banktestutil.NewMockAccountKeeper(ctrl) + authKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes() + + k := keeper.NewBaseKeeper( + encCfg.Codec, + storeService, + authKeeper, + map[string]bool{accAddrs[4].String(): true}, + authtypes.NewModuleAddress("gov").String(), + log.NewNopLogger(), + ) + + // test we can decode balances without problems + // using the old value format of the denom to address index + bankDenomAddressLegacyIndexValue := []byte{0} // taken from: https://github.com/cosmos/cosmos-sdk/blob/v0.47.3/x/bank/keeper/send.go#L361 + rawKey, err := collections.EncodeKeyWithPrefix( + banktypes.DenomAddressPrefix, + k.Balances.Indexes.Denom.KeyCodec(), + collections.Join("atom", sdk.AccAddress("test")), + ) + require.NoError(t, err) + // we set the index key to the old value. + require.NoError(t, storeService.OpenKVStore(ctx).Set(rawKey, bankDenomAddressLegacyIndexValue)) + + // test walking is ok + err = k.Balances.Indexes.Denom.Walk(ctx, nil, func(indexingKey string, indexedKey sdk.AccAddress) (stop bool, err error) { + require.Equal(t, indexedKey, sdk.AccAddress("test")) + require.Equal(t, indexingKey, "atom") + return true, nil + }) + require.NoError(t, err) + + // test matching is also ok + iter, err := k.Balances.Indexes.Denom.MatchExact(ctx, "atom") + require.NoError(t, err) + defer iter.Close() + pks, err := iter.PrimaryKeys() + require.NoError(t, err) + require.Len(t, pks, 1) + require.Equal(t, pks[0], collections.Join(sdk.AccAddress("test"), "atom")) + + // assert the index value will be updated to the new format + err = k.Balances.Indexes.Denom.Reference(ctx, collections.Join(sdk.AccAddress("test"), "atom"), math.ZeroInt(), nil) + require.NoError(t, err) + + newRawValue, err := storeService.OpenKVStore(ctx).Get(rawKey) + require.NoError(t, err) + require.Equal(t, []byte{}, newRawValue) +} diff --git a/x/bank/keeper/view.go b/x/bank/keeper/view.go index 747b85281dde..9132032f7744 100644 --- a/x/bank/keeper/view.go +++ b/x/bank/keeper/view.go @@ -43,6 +43,7 @@ func newBalancesIndexes(sb *collections.SchemaBuilder) BalancesIndexes { Denom: indexes.NewReversePair[math.Int]( sb, types.DenomAddressPrefix, "address_by_denom_index", collections.PairKeyCodec(sdk.LengthPrefixedAddressKey(sdk.AccAddressKey), collections.StringKey), // nolint:staticcheck // Note: refer to the LengthPrefixedAddressKey docs to understand why we do this. + indexes.WithReversePairUncheckedValue(), // denom to address indexes were stored as Key: Join(denom, address) Value: []byte{0}, this will migrate the value to []byte{} in a lazy way. ), } } @@ -81,7 +82,7 @@ func NewBaseViewKeeper(cdc codec.BinaryCodec, storeService store.KVStoreService, Supply: collections.NewMap(sb, types.SupplyKey, "supply", collections.StringKey, sdk.IntValue), DenomMetadata: collections.NewMap(sb, types.DenomMetadataPrefix, "denom_metadata", collections.StringKey, codec.CollValue[types.Metadata](cdc)), SendEnabled: collections.NewMap(sb, types.SendEnabledPrefix, "send_enabled", collections.StringKey, codec.BoolValue), // NOTE: we use a bool value which uses protobuf to retain state backwards compat - Balances: collections.NewIndexedMap(sb, types.BalancesPrefix, "balances", collections.PairKeyCodec(sdk.AccAddressKey, collections.StringKey), types.NewBalanceCompatValueCodec(), newBalancesIndexes(sb)), + Balances: collections.NewIndexedMap(sb, types.BalancesPrefix, "balances", collections.PairKeyCodec(sdk.AccAddressKey, collections.StringKey), types.BalanceValueCodec, newBalancesIndexes(sb)), Params: collections.NewItem(sb, types.ParamsKey, "params", codec.CollValue[types.Params](cdc)), } diff --git a/x/bank/types/keys.go b/x/bank/types/keys.go index 7330459cd238..b4ea683d4b69 100644 --- a/x/bank/types/keys.go +++ b/x/bank/types/keys.go @@ -34,27 +34,13 @@ var ( ParamsKey = collections.NewPrefix(5) ) -// NewBalanceCompatValueCodec is a codec for encoding Balances in a backwards compatible way -// with respect to the old format. -func NewBalanceCompatValueCodec() collcodec.ValueCodec[math.Int] { - return balanceCompatValueCodec{ - sdk.IntValue, - } -} - -type balanceCompatValueCodec struct { - collcodec.ValueCodec[math.Int] -} - -func (v balanceCompatValueCodec) Decode(b []byte) (math.Int, error) { - i, err := v.ValueCodec.Decode(b) - if err == nil { - return i, nil - } +// BalanceValueCodec is a codec for encoding bank balances in a backwards compatible way. +// Historically, balances were represented as Coin, now they're represented as a simple math.Int +var BalanceValueCodec = collcodec.NewAltValueCodec(sdk.IntValue, func(bytes []byte) (math.Int, error) { c := new(sdk.Coin) - err = c.Unmarshal(b) + err := c.Unmarshal(bytes) if err != nil { return math.Int{}, err } return c.Amount, nil -} +}) diff --git a/x/bank/types/keys_test.go b/x/bank/types/keys_test.go index cf0c01eddd62..fa2c48669b61 100644 --- a/x/bank/types/keys_test.go +++ b/x/bank/types/keys_test.go @@ -12,16 +12,15 @@ import ( ) func TestBalanceValueCodec(t *testing.T) { - c := NewBalanceCompatValueCodec() t.Run("value codec implementation", func(t *testing.T) { - colltest.TestValueCodec(t, c, math.NewInt(100)) + colltest.TestValueCodec(t, BalanceValueCodec, math.NewInt(100)) }) t.Run("legacy coin", func(t *testing.T) { coin := sdk.NewInt64Coin("coin", 1000) b, err := coin.Marshal() require.NoError(t, err) - amt, err := c.Decode(b) + amt, err := BalanceValueCodec.Decode(b) require.NoError(t, err) require.Equal(t, coin.Amount, amt) })