diff --git a/CHANGELOG.md b/CHANGELOG.md index 7891301c7c6..e204b5a756a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +* (apps/rate-limiting) [\#8546](https://github.com/cosmos/ibc-go/issues/8546) Fix incorrect `AddressWhitelistKeyPrefix` key name from "address-blacklist" to "address-whitelist". + ### Testing API * [\#8366](https://github.com/cosmos/ibc-go/pull/8366) - Replaced the deprecated `codec.ProtoMarshaler` interface with `proto.Message`. @@ -75,6 +77,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### State Machine Breaking +* (apps/rate-limiting) [\#8546](https://github.com/cosmos/ibc-go/issues/8546) Migrate address whitelist store keys from incorrect "address-blacklist" prefix to correct "address-whitelist" prefix. + ### Improvements * (core/api) [\#8303](https://github.com/cosmos/ibc-go/pull/8303) Prefix-based routing in IBCv2 Router diff --git a/modules/apps/rate-limiting/keeper/migrations.go b/modules/apps/rate-limiting/keeper/migrations.go new file mode 100644 index 00000000000..561930f0444 --- /dev/null +++ b/modules/apps/rate-limiting/keeper/migrations.go @@ -0,0 +1,23 @@ +package keeper + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + + v2 "github.com/cosmos/ibc-go/v10/modules/apps/rate-limiting/migrations/v2" +) + +// Migrator is a struct for handling in-place store migrations. +type Migrator struct { + keeper *Keeper +} + +// NewMigrator returns a new Migrator. +func NewMigrator(keeper *Keeper) Migrator { + return Migrator{keeper: keeper} +} + +// Migrate1to2 migrates the rate-limiting store from v1 to v2 by: +// - Migrating whitelist entries from the incorrect "address-blacklist" prefix to the correct "address-whitelist" prefix +func (m Migrator) Migrate1to2(ctx sdk.Context) error { + return v2.MigrateStore(ctx, m.keeper.storeService, m.keeper.cdc) +} diff --git a/modules/apps/rate-limiting/keeper/v2/store.go b/modules/apps/rate-limiting/keeper/v2/store.go new file mode 100644 index 00000000000..6acfa499f08 --- /dev/null +++ b/modules/apps/rate-limiting/keeper/v2/store.go @@ -0,0 +1,53 @@ +package v2 + +import ( + "cosmossdk.io/core/store" + storetypes "cosmossdk.io/store/types" + + "github.com/cosmos/cosmos-sdk/codec" + "github.com/cosmos/cosmos-sdk/runtime" + sdk "github.com/cosmos/cosmos-sdk/types" + + "github.com/cosmos/ibc-go/v10/modules/apps/rate-limiting/types" +) + +// MigrateStore migrates the rate-limiting store from v1 to v2 by: +// - Migrating whitelist entries from the incorrect "address-blacklist" prefix to the correct "address-whitelist" prefix +func MigrateStore(ctx sdk.Context, storeService store.KVStoreService, cdc codec.BinaryCodec) error { + kvStore := storeService.OpenKVStore(ctx) + return migrateAddressWhitelistKeys(runtime.KVStoreAdapter(kvStore), cdc) +} + +// migrateAddressWhitelistKeys migrates whitelist entries from the legacy key prefix to the correct key prefix +func migrateAddressWhitelistKeys(store storetypes.KVStore, cdc codec.BinaryCodec) error { + // Get all entries with the legacy prefix + iterator := storetypes.KVStorePrefixIterator(store, types.LegacyAddressWhitelistKeyPrefix) + defer iterator.Close() + + // Collect all entries that need to be migrated + var entries []types.WhitelistedAddressPair + var keysToDelete [][]byte + + for ; iterator.Valid(); iterator.Next() { + var whitelist types.WhitelistedAddressPair + if err := cdc.Unmarshal(iterator.Value(), &whitelist); err != nil { + return err + } + entries = append(entries, whitelist) + keysToDelete = append(keysToDelete, iterator.Key()) + } + + // Set entries with the new prefix + for _, whitelist := range entries { + newKey := append(types.AddressWhitelistKeyPrefix, types.AddressWhitelistKey(whitelist.Sender, whitelist.Receiver)...) + value := cdc.MustMarshal(&whitelist) + store.Set(newKey, value) + } + + // Delete old entries + for _, key := range keysToDelete { + store.Delete(key) + } + + return nil +} diff --git a/modules/apps/rate-limiting/keeper/v2/store_test.go b/modules/apps/rate-limiting/keeper/v2/store_test.go new file mode 100644 index 00000000000..eed9b3a34f3 --- /dev/null +++ b/modules/apps/rate-limiting/keeper/v2/store_test.go @@ -0,0 +1,107 @@ +package v2_test + +import ( + "testing" + + testifysuite "github.com/stretchr/testify/suite" + + storetypes "cosmossdk.io/store/types" + + "github.com/cosmos/cosmos-sdk/codec" + "github.com/cosmos/cosmos-sdk/runtime" + + v2 "github.com/cosmos/ibc-go/v10/modules/apps/rate-limiting/migrations/v2" + "github.com/cosmos/ibc-go/v10/modules/apps/rate-limiting/types" + ibctesting "github.com/cosmos/ibc-go/v10/testing" +) + +type MigrationsV2TestSuite struct { + testifysuite.Suite + + coordinator *ibctesting.Coordinator + chainA *ibctesting.TestChain + cdc codec.BinaryCodec +} + +func (suite *MigrationsV2TestSuite) SetupTest() { + suite.coordinator = ibctesting.NewCoordinator(suite.T(), 1) + suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1)) + suite.cdc = suite.chainA.App.AppCodec() +} + +func TestMigrationsV2TestSuite(t *testing.T) { + testifysuite.Run(t, new(MigrationsV2TestSuite)) +} + +func (suite *MigrationsV2TestSuite) TestMigrateStore() { + ctx := suite.chainA.GetContext() + storeService := runtime.NewKVStoreService(suite.chainA.GetSimApp().GetKey(types.StoreKey)) + store := runtime.KVStoreAdapter(storeService.OpenKVStore(ctx)) + + // Create test whitelist entries using the legacy prefix + whitelistPairs := []types.WhitelistedAddressPair{ + { + Sender: "cosmos1abc123", + Receiver: "cosmos1def456", + }, + { + Sender: "cosmos1ghi789", + Receiver: "cosmos1jkl012", + }, + } + + // Store entries with the legacy prefix + for _, pair := range whitelistPairs { + key := append(types.LegacyAddressWhitelistKeyPrefix, types.AddressWhitelistKey(pair.Sender, pair.Receiver)...) + value := suite.cdc.MustMarshal(&pair) + store.Set(key, value) + } + + // Verify entries exist with legacy prefix + for _, pair := range whitelistPairs { + key := append(types.LegacyAddressWhitelistKeyPrefix, types.AddressWhitelistKey(pair.Sender, pair.Receiver)...) + suite.Require().True(store.Has(key)) + } + + // Run migration + err := v2.MigrateStore(ctx, storeService, suite.cdc) + suite.Require().NoError(err) + + // Verify entries no longer exist with legacy prefix + for _, pair := range whitelistPairs { + key := append(types.LegacyAddressWhitelistKeyPrefix, types.AddressWhitelistKey(pair.Sender, pair.Receiver)...) + suite.Require().False(store.Has(key)) + } + + // Verify entries exist with new prefix + for _, pair := range whitelistPairs { + key := append(types.AddressWhitelistKeyPrefix, types.AddressWhitelistKey(pair.Sender, pair.Receiver)...) + suite.Require().True(store.Has(key)) + + // Verify the value is preserved correctly + value := store.Get(key) + var retrievedPair types.WhitelistedAddressPair + suite.cdc.MustUnmarshal(value, &retrievedPair) + suite.Require().Equal(pair.Sender, retrievedPair.Sender) + suite.Require().Equal(pair.Receiver, retrievedPair.Receiver) + } +} + +func (suite *MigrationsV2TestSuite) TestMigrateStoreEmptyStore() { + ctx := suite.chainA.GetContext() + storeService := runtime.NewKVStoreService(suite.chainA.GetSimApp().GetKey(types.StoreKey)) + + // Run migration on empty store + err := v2.MigrateStore(ctx, storeService, suite.cdc) + suite.Require().NoError(err) + + // Verify no entries exist + store := runtime.KVStoreAdapter(storeService.OpenKVStore(ctx)) + iterator := storetypes.KVStorePrefixIterator(store, types.LegacyAddressWhitelistKeyPrefix) + defer iterator.Close() + suite.Require().False(iterator.Valid()) + + iterator = storetypes.KVStorePrefixIterator(store, types.AddressWhitelistKeyPrefix) + defer iterator.Close() + suite.Require().False(iterator.Valid()) +} diff --git a/modules/apps/rate-limiting/module.go b/modules/apps/rate-limiting/module.go index 5834ad09b07..4de9cab0fd8 100644 --- a/modules/apps/rate-limiting/module.go +++ b/modules/apps/rate-limiting/module.go @@ -108,6 +108,11 @@ func NewAppModule(k keeper.Keeper) AppModule { func (am AppModule) RegisterServices(cfg module.Configurator) { types.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServerImpl(am.keeper)) // Use the msgServer implementation types.RegisterQueryServer(cfg.QueryServer(), am.keeper) + + migrator := keeper.NewMigrator(&am.keeper) + if err := cfg.RegisterMigration(types.ModuleName, 1, migrator.Migrate1to2); err != nil { + panic(fmt.Sprintf("failed to migrate rate-limiting from version 1 to 2: %v", err)) + } } // InitGenesis performs genesis initialization for the rate-limiting module. It returns @@ -126,7 +131,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw } // ConsensusVersion implements AppModule/ConsensusVersion defining the current version of rate-limiting. -func (AppModule) ConsensusVersion() uint64 { return 1 } +func (AppModule) ConsensusVersion() uint64 { return 2 } // BeginBlock implements the AppModule interface func (am AppModule) BeginBlock(ctx context.Context) error { diff --git a/modules/apps/rate-limiting/types/keys.go b/modules/apps/rate-limiting/types/keys.go index 1ba017419bd..ea105b97c9c 100644 --- a/modules/apps/rate-limiting/types/keys.go +++ b/modules/apps/rate-limiting/types/keys.go @@ -26,11 +26,14 @@ var ( RateLimitKeyPrefix = bytes("rate-limit") PendingSendPacketPrefix = bytes("pending-send-packet") DenomBlacklistKeyPrefix = bytes("denom-blacklist") - // TODO: Fix IBCGO-2368 - AddressWhitelistKeyPrefix = bytes("address-blacklist") + // Fixed IBCGO-2368: Changed from "address-blacklist" to "address-whitelist" + AddressWhitelistKeyPrefix = bytes("address-whitelist") HourEpochKey = bytes("hour-epoch") PendingSendPacketChannelLength = 16 + + // Legacy key for migration purposes + LegacyAddressWhitelistKeyPrefix = bytes("address-blacklist") ) // Get the rate limit byte key built from the denom and channelId