fix(auth): support legacy global AccountNumber when query historical state (port #23743) (#24533)

Co-authored-by: HuangYi <[email protected]>
Co-authored-by: Alex | Interchain Labs <[email protected]>

Author: 3 people

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

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
 }
 

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)
+		})
+	}
+}

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)

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)

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")
 )