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 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 29 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,39 @@
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)

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
129 changes: 119 additions & 10 deletions x/auth/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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

Expand All @@ -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(),
Expand Down Expand Up @@ -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)
})
}
}
4 changes: 3 additions & 1 deletion x/auth/migrations/v5/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
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