From 3b90ba4ef3da046367dc61248e7e4df1e27f5886 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Wed, 23 Apr 2025 22:53:36 +0800 Subject: [PATCH] fix(auth): support legacy global AccountNumber when query historical state (port #23743) (#24533) Co-authored-by: HuangYi Co-authored-by: Alex | Interchain Labs --- CHANGELOG.md | 1 + x/auth/keeper/keeper.go | 30 ++++++- x/auth/keeper/keeper_test.go | 129 ++++++++++++++++++++++++--- x/auth/migrations/v5/migrate.go | 4 +- x/auth/migrations/v5/migrate_test.go | 4 +- x/auth/types/keys.go | 3 + 6 files changed, 158 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f63f32c157..2a802367b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -105,6 +105,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (baseapp) [#24074](https://github.com/cosmos/cosmos-sdk/pull/24074) Use CometBFT's ComputeProtoSizeForTxs in defaultTxSelector.SelectTxForProposal for consistency. * (cli) [#24090](https://github.com/cosmos/cosmos-sdk/pull/24090) Prune cmd should disable async pruning. * (x/auth) [#19239](https://github.com/cosmos/cosmos-sdk/pull/19239) Sets from flag in multi-sign command to avoid no key name provided error. +* (x/auth) [#23741](https://github.com/cosmos/cosmos-sdk/pull/23741) Support legacy global AccountNumber for legacy compatibility. * (baseapp) [#24526](https://github.com/cosmos/cosmos-sdk/pull/24526) Fix incorrect retention height when `commitHeight` equals `minRetainBlocks`. ## [v0.50.12](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.12) - 2025-02-20 diff --git a/x/auth/keeper/keeper.go b/x/auth/keeper/keeper.go index f03f45af8a..2fb009bb98 100644 --- a/x/auth/keeper/keeper.go +++ b/x/auth/keeper/keeper.go @@ -6,6 +6,8 @@ import ( "fmt" "time" + gogotypes "github.com/cosmos/gogoproto/types" + "cosmossdk.io/collections" "cosmossdk.io/collections/indexes" "cosmossdk.io/core/address" @@ -181,13 +183,39 @@ func (ak AccountKeeper) GetSequence(ctx context.Context, addr sdk.AccAddress) (u return acc.GetSequence(), nil } +func (ak AccountKeeper) getAccountNumberLegacy(ctx context.Context) (uint64, error) { + store := ak.storeService.OpenKVStore(ctx) + b, err := store.Get(types.LegacyGlobalAccountNumberKey) + if err != nil { + return 0, fmt.Errorf("failed to get legacy account number: %w", err) + } + v := new(gogotypes.UInt64Value) + if err := v.Unmarshal(b); err != nil { + return 0, fmt.Errorf("failed to unmarshal legacy account number: %w", err) + } + return v.Value, nil +} + // NextAccountNumber returns and increments the global account number counter. // If the global account number is not set, it initializes it with value 0. func (ak AccountKeeper) NextAccountNumber(ctx context.Context) uint64 { - n, err := ak.AccountNumber.Next(ctx) + n, err := collections.Item[uint64](ak.AccountNumber).Get(ctx) + if err != nil && errors.Is(err, collections.ErrNotFound) { + // this won't happen in the tip of production network, + // but can happen when query historical states, + // fallback to old key for backward-compatibility. + // for more info, see https://github.com/cosmos/cosmos-sdk/issues/23741 + n, err = ak.getAccountNumberLegacy(ctx) + } + if err != nil { panic(err) } + + if err := ak.AccountNumber.Set(ctx, n+1); err != nil { + panic(err) + } + return n } diff --git a/x/auth/keeper/keeper_test.go b/x/auth/keeper/keeper_test.go index 8eff83d1dd..803ab90948 100644 --- a/x/auth/keeper/keeper_test.go +++ b/x/auth/keeper/keeper_test.go @@ -3,9 +3,15 @@ package keeper_test import ( "testing" + cmtproto "github.com/cometbft/cometbft/proto/tendermint/types" + cmttime "github.com/cometbft/cometbft/types/time" + gogotypes "github.com/cosmos/gogoproto/types" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + "cosmossdk.io/collections" "cosmossdk.io/core/header" + "cosmossdk.io/core/store" storetypes "cosmossdk.io/store/types" "github.com/cosmos/cosmos-sdk/baseapp" @@ -32,6 +38,17 @@ var ( randomPermAcc = types.NewEmptyModuleAccount(randomPerm, "random") ) +func getMaccPerms() map[string][]string { + return map[string][]string{ + "fee_collector": nil, + "mint": {"minter"}, + "bonded_tokens_pool": {"burner", "staking"}, + "not_bonded_tokens_pool": {"burner", "staking"}, + multiPerm: {"burner", "minter", "staking"}, + randomPerm: {"random"}, + } +} + type KeeperTestSuite struct { suite.Suite @@ -51,20 +68,11 @@ func (suite *KeeperTestSuite) SetupTest() { testCtx := testutil.DefaultContextWithDB(suite.T(), key, storetypes.NewTransientStoreKey("transient_test")) suite.ctx = testCtx.Ctx.WithHeaderInfo(header.Info{}) - maccPerms := map[string][]string{ - "fee_collector": nil, - "mint": {"minter"}, - "bonded_tokens_pool": {"burner", "staking"}, - "not_bonded_tokens_pool": {"burner", "staking"}, - multiPerm: {"burner", "minter", "staking"}, - randomPerm: {"random"}, - } - suite.accountKeeper = keeper.NewAccountKeeper( suite.encCfg.Codec, storeService, types.ProtoBaseAccount, - maccPerms, + getMaccPerms(), authcodec.NewBech32Codec("cosmos"), "cosmos", types.NewModuleAddress("gov").String(), @@ -215,3 +223,104 @@ func (suite *KeeperTestSuite) TestInitGenesis() { // we expect nextNum to be 2 because we initialize fee_collector as account number 1 suite.Require().Equal(2, int(nextNum)) } + +func setupAccountKeeper(t *testing.T) (sdk.Context, keeper.AccountKeeper, store.KVStoreService) { + t.Helper() + key := storetypes.NewKVStoreKey(types.StoreKey) + storeService := runtime.NewKVStoreService(key) + testCtx := testutil.DefaultContextWithDB(t, key, storetypes.NewTransientStoreKey("transient_test")) + ctx := testCtx.Ctx.WithBlockHeader(cmtproto.Header{Time: cmttime.Now()}) + encCfg := moduletestutil.MakeTestEncodingConfig() + + ak := keeper.NewAccountKeeper( + encCfg.Codec, + storeService, + types.ProtoBaseAccount, + getMaccPerms(), + authcodec.NewBech32Codec("cosmos"), + "cosmos", + types.NewModuleAddress("gov").String(), + ) + + return ctx, ak, storeService +} + +func TestNextAccountNumber(t *testing.T) { + const newNum = uint64(100) + const legacyNum = uint64(50) + legacyVal := &gogotypes.UInt64Value{Value: legacyNum} + ctx, ak, storeService := setupAccountKeeper(t) + testCases := []struct { + name string + setup func() + onNext func() + expects []uint64 + }{ + { + name: "reset account number to 0 after using legacy key", + setup: func() { + data, err := legacyVal.Marshal() + require.NoError(t, err) + store := storeService.OpenKVStore(ctx) + err = store.Set(types.LegacyGlobalAccountNumberKey, data) + require.NoError(t, err) + }, + onNext: func() { + num := uint64(0) + err := ak.AccountNumber.Set(ctx, num) + require.NoError(t, err) + }, + expects: []uint64{legacyNum, 0}, + }, + { + name: "no keys set, account number starts at 0", + setup: func() {}, + expects: []uint64{0, 1}, + }, + { + name: "fallback to legacy key when new key is unset", + setup: func() { + data, err := legacyVal.Marshal() + require.NoError(t, err) + store := storeService.OpenKVStore(ctx) + err = store.Set(types.LegacyGlobalAccountNumberKey, data) + require.NoError(t, err) + + // unset new key + err = (collections.Item[uint64])(ak.AccountNumber).Remove(ctx) + require.NoError(t, err) + }, + expects: []uint64{legacyNum, legacyNum + 1}, + }, + { + name: "new key takes precedence over legacy key", + setup: func() { + data, err := legacyVal.Marshal() + require.NoError(t, err) + store := storeService.OpenKVStore(ctx) + err = store.Set(types.LegacyGlobalAccountNumberKey, data) + require.NoError(t, err) + + err = ak.AccountNumber.Set(ctx, newNum) + require.NoError(t, err) + }, + expects: []uint64{newNum, newNum + 1}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctx, ak, storeService = setupAccountKeeper(t) + tc.setup() + nextNum := ak.NextAccountNumber(ctx) + require.Equal(t, tc.expects[0], nextNum) + + if tc.onNext != nil { + tc.onNext() + } + + nextNum = ak.NextAccountNumber(ctx) + require.Equal(t, tc.expects[1], nextNum) + }) + } +} diff --git a/x/auth/migrations/v5/migrate.go b/x/auth/migrations/v5/migrate.go index 44fcf970c3..7b5552066d 100644 --- a/x/auth/migrations/v5/migrate.go +++ b/x/auth/migrations/v5/migrate.go @@ -7,9 +7,11 @@ import ( "cosmossdk.io/collections" storetypes "cosmossdk.io/core/store" + + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" ) -var LegacyGlobalAccountNumberKey = []byte("globalAccountNumber") +var LegacyGlobalAccountNumberKey = authtypes.LegacyGlobalAccountNumberKey func Migrate(ctx context.Context, storeService storetypes.KVStoreService, sequence collections.Sequence) error { store := storeService.OpenKVStore(ctx) diff --git a/x/auth/migrations/v5/migrate_test.go b/x/auth/migrations/v5/migrate_test.go index c0c7113fb5..81f7709d9d 100644 --- a/x/auth/migrations/v5/migrate_test.go +++ b/x/auth/migrations/v5/migrate_test.go @@ -8,6 +8,8 @@ import ( "cosmossdk.io/collections" "cosmossdk.io/collections/colltest" + + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" ) func TestMigrate(t *testing.T) { @@ -21,7 +23,7 @@ func TestMigrate(t *testing.T) { legacySeqBytes, err := (&types.UInt64Value{Value: wantValue}).Marshal() require.NoError(t, err) - err = kv.OpenKVStore(ctx).Set(LegacyGlobalAccountNumberKey, legacySeqBytes) + err = kv.OpenKVStore(ctx).Set(authtypes.LegacyGlobalAccountNumberKey, legacySeqBytes) require.NoError(t, err) err = Migrate(ctx, kv, seq) diff --git a/x/auth/types/keys.go b/x/auth/types/keys.go index 0f96301972..5e28ee49b0 100644 --- a/x/auth/types/keys.go +++ b/x/auth/types/keys.go @@ -31,4 +31,7 @@ var ( // UnorderedNoncesKey prefix for the unordered sequence storage. UnorderedNoncesKey = collections.NewPrefix(90) + + // legacy param key for global account number + LegacyGlobalAccountNumberKey = []byte("globalAccountNumber") )