Skip to content

fix(apps/rate-limiting): correct AddressWhitelistKeyPrefix from "address-blacklist" to "address-whitelist" #8554

New issue

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

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

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -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
Expand Down
23 changes: 23 additions & 0 deletions modules/apps/rate-limiting/keeper/migrations.go
Original file line number Diff line number Diff line change
@@ -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)
}
53 changes: 53 additions & 0 deletions modules/apps/rate-limiting/keeper/v2/store.go
Original file line number Diff line number Diff line change
@@ -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
}
107 changes: 107 additions & 0 deletions modules/apps/rate-limiting/keeper/v2/store_test.go
Original file line number Diff line number Diff line change
@@ -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())
}
7 changes: 6 additions & 1 deletion modules/apps/rate-limiting/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
7 changes: 5 additions & 2 deletions modules/apps/rate-limiting/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down