Skip to content

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

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

Merged
merged 16 commits into from
Apr 23, 2025
Merged
Show file tree
Hide file tree
Changes from 3 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,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

## [v0.50.12](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.12) - 2025-02-20

Expand Down
29 changes: 28 additions & 1 deletion x/auth/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
"fmt"
"time"

gogotypes "github.com/cosmos/gogoproto/types"

"cosmossdk.io/collections"
"cosmossdk.io/collections/indexes"
"cosmossdk.io/core/address"
Expand Down Expand Up @@ -181,13 +183,38 @@
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.
n, err = ak.getAccountNumberLegacy(ctx)
}

if err != nil {
panic(err)
}

if err := ak.AccountNumber.Set(ctx, n+1); err != nil {
panic(err)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
}

return n
}

Expand Down
64 changes: 54 additions & 10 deletions x/auth/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ 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/core/header"
Expand Down Expand Up @@ -32,6 +36,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

Expand All @@ -51,20 +66,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(),
Expand Down Expand Up @@ -215,3 +221,41 @@ 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 TestNextAccountNumber(t *testing.T) {
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(),
)

num := uint64(10)
val := &gogotypes.UInt64Value{
Value: num,
}
data, err := val.Marshal()
require.NoError(t, err)
store := storeService.OpenKVStore(ctx)
err = store.Set(types.LegacyGlobalAccountNumberKey, data)
require.NoError(t, err)

nextNum := ak.NextAccountNumber(ctx)
require.Equal(t, num, nextNum)
Copy link
Contributor

@aljo242 aljo242 Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be its own t.Run(...) testcase with a description?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

execute order matters, seems separate case is better


num = uint64(0)
err = ak.AccountNumber.Set(ctx, num)
require.NoError(t, err)

nextNum = ak.NextAccountNumber(ctx)
require.Equal(t, num, nextNum)
}
8 changes: 4 additions & 4 deletions x/auth/migrations/v5/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ import (

"cosmossdk.io/collections"
storetypes "cosmossdk.io/core/store"
)

var LegacyGlobalAccountNumberKey = []byte("globalAccountNumber")
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
)

func Migrate(ctx context.Context, storeService storetypes.KVStoreService, sequence collections.Sequence) error {
store := storeService.OpenKVStore(ctx)
b, err := store.Get(LegacyGlobalAccountNumberKey)
b, err := store.Get(authtypes.LegacyGlobalAccountNumberKey)
if err != nil {
return err
}
Expand All @@ -37,7 +37,7 @@ func Migrate(ctx context.Context, storeService storetypes.KVStoreService, sequen
}

// remove the value from the old prefix.
err = store.Delete(LegacyGlobalAccountNumberKey)
err = store.Delete(authtypes.LegacyGlobalAccountNumberKey)
if err != nil {
return err
}
Expand Down
4 changes: 3 additions & 1 deletion x/auth/migrations/v5/migrate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions x/auth/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
)
Loading