From 0422760c6ad2b1d63391ecaeec3fbd43c4716632 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Pernas=20Maradei?= Date: Tue, 1 Aug 2023 17:23:09 +0200 Subject: [PATCH] Revert "fix(bank): make `DenomAddressIndex` less strict with respect to index values. (backport #16841) (#16853)" This reverts commit c919775fdbdfe6629aa02a113eeb5c479d776102. --- CHANGELOG.md | 40 +++++++++++++++ x/bank/keeper/collections_test.go | 85 ------------------------------- x/bank/keeper/view.go | 3 +- x/bank/types/keys.go | 24 +++++++-- x/bank/types/keys_test.go | 5 +- 5 files changed, 63 insertions(+), 94 deletions(-) delete mode 100644 x/bank/keeper/collections_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index a08c1aa86e29..1f6be6ff8c23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,7 +38,47 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +<<<<<<< HEAD ## [v0.50.0-beta.0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.0-beta.0) - 2023-07-19 +======= +### API Breaking Changes + +* (types/math) [#16040](https://github.com/cosmos/cosmos-sdk/pull/16798) Remove aliases in `types/math.go` (part 2). +* (x/staking) [#16795](https://github.com/cosmos/cosmos-sdk/pull/16795) `DelegationToDelegationResponse`, `DelegationsToDelegationResponses`, `RedelegationsToRedelegationResponses` are no longer exported. + +## [v0.50.0-alpha.1](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.0-alpha.1) - 2023-06-30 + +### Features + +* (sims) [#16656](https://github.com/cosmos/cosmos-sdk/pull/16656) Add custom max gas for block for sim config with unlimited as default. + +### Improvements + +* (all) [#16497](https://github.com/cosmos/cosmos-sdk/pull/16497) Removed all exported vestiges of `sdk.MustSortJSON` and `sdk.SortJSON`. +* (x/distribution) [#16218](https://github.com/cosmos/cosmos-sdk/pull/16218) Add Autocli config to distribution module. +* (cli) [#16206](https://github.com/cosmos/cosmos-sdk/pull/16206) Make ABCI handshake profileable. + +### Bug Fixes + +* (x/consensus) [#16713](https://github.com/cosmos/cosmos-sdk/pull/16713) Add missing ABCI param in MsgUpdateParams. +* [#16547](https://github.com/cosmos/cosmos-sdk/pull/16547) Ensure a transaction's gas limit cannot exceed the block gas limit. +* (x/auth) [#16554](https://github.com/cosmos/cosmos-sdk/pull/16554) `ModuleAccount.Validate` now reports a nil `.BaseAccount` instead of panicking. +* (baseapp) [#16613](https://github.com/cosmos/cosmos-sdk/pull/16613) Ensure each message in a transaction has a registered handler, otherwise `CheckTx` will fail. +* [#16639](https://github.com/cosmos/cosmos-sdk/pull/16639) Make sure we don't execute blocks beyond the halt height. +* (x/auth/vesting) [#16733](https://github.com/cosmos/cosmos-sdk/pull/16733) Panic on overflowing and negative EndTimes when creating a PeriodicVestingAccount. +* (baseapp) [#16700](https://github.com/cosmos/cosmos-sdk/pull/16700) Fix consensus failure in returning no response to malformed transactions. +* (baseapp) [#16596](https://github.com/cosmos/cosmos-sdk/pull/16596) Return error during ExtendVote and VerifyVoteExtension if the request height is earlier than `VoteExtensionsEnableHeight`. +* (x/slashing) [#16784](https://github.com/cosmos/cosmos-sdk/pull/16784) Emit event with the correct reason in SlashWithInfractionReason. + +### API Breaking Changes + +* (x/staking) [#16324](https://github.com/cosmos/cosmos-sdk/pull/16324) `NewKeeper` now takes a `KVStoreService` instead of a `StoreKey`, and methods in the `Keeper` now take a `context.Context` instead of a `sdk.Context` and return an `error`. Notable changes: + * `Validator` method now returns `types.ErrNoValidatorFound` instead of `nil` when not found. +* (x/auth) [#16621](https://github.com/cosmos/cosmos-sdk/pull/16621), [#16768](https://github.com/cosmos/cosmos-sdk/pull/16768) Pass address codecs to auth new keeper constructor. +* (x/auth/vesting) [#16741](https://github.com/cosmos/cosmos-sdk/pull/16741) Vesting account constructor now return an error with the result of their validate function. + +## [v0.50.0-alpha.0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.0-alpha.0) - 2023-06-07 +>>>>>>> parent of c919775fdb (fix(bank): make `DenomAddressIndex` less strict with respect to index values. (backport #16841) (#16853)) ### Features diff --git a/x/bank/keeper/collections_test.go b/x/bank/keeper/collections_test.go deleted file mode 100644 index e1af343cce56..000000000000 --- a/x/bank/keeper/collections_test.go +++ /dev/null @@ -1,85 +0,0 @@ -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 9132032f7744..747b85281dde 100644 --- a/x/bank/keeper/view.go +++ b/x/bank/keeper/view.go @@ -43,7 +43,6 @@ 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. ), } } @@ -82,7 +81,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.BalanceValueCodec, newBalancesIndexes(sb)), + Balances: collections.NewIndexedMap(sb, types.BalancesPrefix, "balances", collections.PairKeyCodec(sdk.AccAddressKey, collections.StringKey), types.NewBalanceCompatValueCodec(), 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 b4ea683d4b69..7330459cd238 100644 --- a/x/bank/types/keys.go +++ b/x/bank/types/keys.go @@ -34,13 +34,27 @@ var ( ParamsKey = collections.NewPrefix(5) ) -// 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) { +// 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 + } c := new(sdk.Coin) - err := c.Unmarshal(bytes) + err = c.Unmarshal(b) 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 fa2c48669b61..cf0c01eddd62 100644 --- a/x/bank/types/keys_test.go +++ b/x/bank/types/keys_test.go @@ -12,15 +12,16 @@ import ( ) func TestBalanceValueCodec(t *testing.T) { + c := NewBalanceCompatValueCodec() t.Run("value codec implementation", func(t *testing.T) { - colltest.TestValueCodec(t, BalanceValueCodec, math.NewInt(100)) + colltest.TestValueCodec(t, c, 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 := BalanceValueCodec.Decode(b) + amt, err := c.Decode(b) require.NoError(t, err) require.Equal(t, coin.Amount, amt) })