diff --git a/CHANGELOG.md b/CHANGELOG.md index e5d1ce7be6b7..d7db2a3c2b8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -200,6 +200,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i * (simapp) [#19146](https://github.com/cosmos/cosmos-sdk/pull/19146) Replace `--v` CLI option with `--validator-count`/`-n`. * (module) [#19370](https://github.com/cosmos/cosmos-sdk/pull/19370) Deprecate `module.Configurator`, use `appmodule.HasMigrations` and `appmodule.HasServices` instead from Core API. +* (x/auth) [#20531](https://github.com/cosmos/cosmos-sdk/pull/20531) Deprecate auth keeper `NextAccountNumber`, use `keeper.AccountsModKeeper.NextAccountNumber` instead. ## [v0.50.7](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.7) - 2024-06-04 diff --git a/UPGRADING.md b/UPGRADING.md index 48e2db4164c4..e99c120b0519 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -252,6 +252,19 @@ Most of Cosmos SDK modules have migrated to [collections](https://docs.cosmos.ne Many functions have been removed due to this changes as the API can be smaller thanks to collections. For modules that have migrated, verify you are checking against `collections.ErrNotFound` when applicable. +#### `x/accounts` + +Accounts's AccountNumber will be used as a global account number tracking replacing Auth legacy AccountNumber. Must set accounts's AccountNumber with auth's AccountNumber value in upgrade handler. This is done through auth keeper MigrateAccountNumber function. + +```go +import authkeeper "cosmossdk.io/x/auth/keeper" +... +err := authkeeper.MigrateAccountNumberUnsafe(ctx, &app.AuthKeeper) +if err != nil { + return nil, err +} +``` + #### `x/auth` Auth was spun out into its own `go.mod`. To import it use `cosmossdk.io/x/auth` diff --git a/crypto/codec/amino.go b/crypto/codec/amino.go index 648637da276b..373ef14621f8 100644 --- a/crypto/codec/amino.go +++ b/crypto/codec/amino.go @@ -4,6 +4,7 @@ import ( "github.com/cometbft/cometbft/crypto/sr25519" "cosmossdk.io/core/legacy" + bls12_381 "github.com/cosmos/cosmos-sdk/crypto/keys/bls12_381" "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" diff --git a/crypto/codec/cmt.go b/crypto/codec/cmt.go index 9f6eccfdf88d..9617f94bf1a3 100644 --- a/crypto/codec/cmt.go +++ b/crypto/codec/cmt.go @@ -6,6 +6,7 @@ import ( "github.com/cometbft/cometbft/crypto/encoding" "cosmossdk.io/errors" + bls12_381 "github.com/cosmos/cosmos-sdk/crypto/keys/bls12_381" "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" diff --git a/simapp/upgrades.go b/simapp/upgrades.go index 01fe745d6e88..9433b94e37b4 100644 --- a/simapp/upgrades.go +++ b/simapp/upgrades.go @@ -6,6 +6,7 @@ import ( "cosmossdk.io/core/appmodule" storetypes "cosmossdk.io/store/types" "cosmossdk.io/x/accounts" + authkeeper "cosmossdk.io/x/auth/keeper" epochstypes "cosmossdk.io/x/epochs/types" protocolpooltypes "cosmossdk.io/x/protocolpool/types" upgradetypes "cosmossdk.io/x/upgrade/types" @@ -25,6 +26,12 @@ func (app SimApp) RegisterUpgradeHandlers() { app.UpgradeKeeper.SetUpgradeHandler( UpgradeName, func(ctx context.Context, _ upgradetypes.Plan, fromVM appmodule.VersionMap) (appmodule.VersionMap, error) { + // sync accounts and auth module account number + err := authkeeper.MigrateAccountNumberUnsafe(ctx, &app.AuthKeeper) + if err != nil { + return nil, err + } + return app.ModuleManager.RunMigrations(ctx, app.Configurator(), fromVM) }, ) diff --git a/simapp/upgrades_test.go b/simapp/upgrades_test.go new file mode 100644 index 000000000000..b70ee15c7863 --- /dev/null +++ b/simapp/upgrades_test.go @@ -0,0 +1,54 @@ +package simapp + +import ( + "testing" + + cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1" + "github.com/stretchr/testify/require" + + "cosmossdk.io/collections" + authkeeper "cosmossdk.io/x/auth/keeper" + authtypes "cosmossdk.io/x/auth/types" +) + +// TestSyncAccountNumber tests if accounts module account number is set correctly with the value get from auth. +// Also check if the store entry for auth GlobalAccountNumberKey is successfully deleted. +func TestSyncAccountNumber(t *testing.T) { + app := Setup(t, true) + ctx := app.NewUncachedContext(true, cmtproto.Header{}) + + bytesKey := authtypes.GlobalAccountNumberKey + store := app.AuthKeeper.KVStoreService.OpenKVStore(ctx) + + // initially there is no value set yet + v, err := store.Get(bytesKey) + require.NoError(t, err) + require.Nil(t, v) + + // set value for legacy account number + v, err = collections.Uint64Value.Encode(10) + require.NoError(t, err) + err = store.Set(bytesKey, v) + require.NoError(t, err) + + // make sure value are updated + v, err = store.Get(bytesKey) + require.NoError(t, err) + require.NotEmpty(t, v) + num, err := collections.Uint64Value.Decode(v) + require.NoError(t, err) + require.Equal(t, uint64(10), num) + + err = authkeeper.MigrateAccountNumberUnsafe(ctx, &app.AuthKeeper) + require.NoError(t, err) + + // make sure the DB entry for this key is deleted + v, err = store.Get(bytesKey) + require.NoError(t, err) + require.Nil(t, v) + + // check if accounts's account number is updated + currentNum, err := app.AccountsKeeper.AccountNumber.Peek(ctx) + require.NoError(t, err) + require.Equal(t, uint64(10), currentNum) +} diff --git a/x/accounts/genesis.go b/x/accounts/genesis.go index bd0f9edc0b4d..5f9ad3ee954e 100644 --- a/x/accounts/genesis.go +++ b/x/accounts/genesis.go @@ -64,19 +64,28 @@ func (k Keeper) exportAccount(ctx context.Context, addr []byte, accType string, } func (k Keeper) ImportState(ctx context.Context, genState *v1.GenesisState) error { - err := k.AccountNumber.Set(ctx, genState.AccountNumber) - if err != nil { - return err - } - + var largestNum *uint64 + var err error // import accounts for _, acc := range genState.Accounts { err = k.importAccount(ctx, acc) if err != nil { return fmt.Errorf("%w: %s", err, acc.Address) } + + accNum := acc.AccountNumber + + if largestNum == nil || *largestNum < accNum { + largestNum = &accNum + } } - return nil + + if largestNum != nil { + // set the account number to the highest account number to avoid duplicate account number + err = k.AccountNumber.Set(ctx, *largestNum) + } + + return err } func (k Keeper) importAccount(ctx context.Context, acc *v1.GenesisAccount) error { diff --git a/x/accounts/genesis_test.go b/x/accounts/genesis_test.go index 357fb8eb6628..ecd44a91d00d 100644 --- a/x/accounts/genesis_test.go +++ b/x/accounts/genesis_test.go @@ -51,6 +51,23 @@ func TestGenesis(t *testing.T) { resp, err = k.Query(ctx, addr2, &types.DoubleValue{}) require.NoError(t, err) require.Equal(t, &types.UInt64Value{Value: 20}, resp) + + // reset state + k, ctx = newKeeper(t, func(deps implementation.Dependencies) (string, implementation.Account, error) { + acc, err := NewTestAccount(deps) + return testAccountType, acc, err + }) + + // modify the accounts account number + state.Accounts[0].AccountNumber = 99 + + err = k.ImportState(ctx, state) + require.NoError(t, err) + + currentAccNum, err := k.AccountNumber.Peek(ctx) + require.NoError(t, err) + // AccountNumber should be set to the highest account number in the genesis state + require.Equal(t, uint64(99), currentAccNum) } func TestImportAccountError(t *testing.T) { diff --git a/x/auth/keeper/keeper.go b/x/auth/keeper/keeper.go index 7c86fb093390..00240e8f9a65 100644 --- a/x/auth/keeper/keeper.go +++ b/x/auth/keeper/keeper.go @@ -102,6 +102,10 @@ type AccountKeeper struct { Schema collections.Schema Params collections.Item[types.Params] + // only use for upgrade handler + // + // Deprecated: move to accounts module accountNumber + accountNumber collections.Sequence // Accounts key: AccAddr | value: AccountI | index: AccountsIndex Accounts *collections.IndexedMap[sdk.AccAddress, sdk.AccountI, AccountsIndexes] } @@ -135,6 +139,7 @@ func NewAccountKeeper( permAddrs: permAddrs, authority: authority, Params: collections.NewItem(sb, types.ParamsKey, "params", codec.CollValue[types.Params](cdc)), + accountNumber: collections.NewSequence(sb, types.GlobalAccountNumberKey, "account_number"), Accounts: collections.NewIndexedMap(sb, types.AddressStoreKeyPrefix, "accounts", sdk.AccAddressKey, codec.CollInterfaceValue[sdk.AccountI](cdc), NewAccountIndexes(sb)), } schema, err := sb.Build() @@ -145,6 +150,22 @@ func NewAccountKeeper( return ak } +// removeLegacyAccountNumberUnsafe is used for migration purpose only. It deletes the sequence in the DB +// and returns the last value used on success. +// Deprecated +func (ak AccountKeeper) removeLegacyAccountNumberUnsafe(ctx context.Context) (uint64, error) { + accNum, err := ak.accountNumber.Peek(ctx) + if err != nil { + return 0, err + } + + // Delete DB entry for legacy account number + store := ak.KVStoreService.OpenKVStore(ctx) + err = store.Delete(types.GlobalAccountNumberKey.Bytes()) + + return accNum, err +} + // GetAuthority returns the x/auth module's authority. func (ak AccountKeeper) GetAuthority() string { return ak.authority @@ -317,3 +338,18 @@ func (ak AccountKeeper) NonAtomicMsgsExec(ctx context.Context, signer sdk.AccAdd return msgResponses, nil } + +// MigrateAccountNumberUnsafe migrates auth's account number to accounts's account number +// and delete store entry for auth legacy GlobalAccountNumberKey. +// +// Should only use in an upgrade handler for migrating account number. +func MigrateAccountNumberUnsafe(ctx context.Context, ak *AccountKeeper) error { + currentAccNum, err := ak.removeLegacyAccountNumberUnsafe(ctx) + if err != nil { + return fmt.Errorf("failed to migrate account number: %w", err) + } + + err = ak.AccountsModKeeper.InitAccountNumberSeqUnsafe(ctx, currentAccNum) + + return err +} diff --git a/x/auth/keeper/migrations.go b/x/auth/keeper/migrations.go index 5b3418b99e65..c858e7e9551a 100644 --- a/x/auth/keeper/migrations.go +++ b/x/auth/keeper/migrations.go @@ -3,7 +3,6 @@ package keeper import ( "context" - "cosmossdk.io/collections" v5 "cosmossdk.io/x/auth/migrations/v5" "cosmossdk.io/x/auth/types" @@ -13,16 +12,11 @@ import ( // Migrator is a struct for handling in-place store migrations. type Migrator struct { keeper AccountKeeper - // accNum is use in v4 to v5 and v5 to v6 migration - accNum collections.Sequence } // NewMigrator returns a new Migrator. func NewMigrator(keeper AccountKeeper) Migrator { - sb := collections.NewSchemaBuilder(keeper.Environment.KVStoreService) - accNumSeq := collections.NewSequence(sb, types.GlobalAccountNumberKey, "account_number") - - return Migrator{keeper: keeper, accNum: accNumSeq} + return Migrator{keeper: keeper} } // Migrate1to2 migrates from version 1 to 2. @@ -48,18 +42,7 @@ func (m Migrator) Migrate3to4(ctx context.Context) error { // It migrates the GlobalAccountNumber from being a protobuf defined value to a // big-endian encoded uint64, it also migrates it to use a more canonical prefix. func (m Migrator) Migrate4To5(ctx context.Context) error { - return v5.Migrate(ctx, m.keeper.KVStoreService, m.accNum) -} - -// Migrate5To6 migrates the x/auth module state from the consensus version 5 to 6. -// It migrates the GlobalAccountNumber from x/auth to x/accounts . -func (m Migrator) Migrate5To6(ctx context.Context) error { - currentAccNum, err := m.accNum.Peek(ctx) - if err != nil { - return err - } - - return m.keeper.AccountsModKeeper.InitAccountNumberSeqUnsafe(ctx, currentAccNum) + return v5.Migrate(ctx, m.keeper.KVStoreService, m.keeper.accountNumber) } // V45SetAccount implements V45_SetAccount diff --git a/x/auth/migrations/v6/migrate.go b/x/auth/migrations/v6/migrate.go deleted file mode 100644 index 889de0e9c9e4..000000000000 --- a/x/auth/migrations/v6/migrate.go +++ /dev/null @@ -1,12 +0,0 @@ -package v6 - -import ( - "context" -) - -type migrateAccNumFunc = func(ctx context.Context) error - -// Migrate account number from x/auth account number to x/accounts account number -func Migrate(ctx context.Context, f migrateAccNumFunc) error { - return f(ctx) -} diff --git a/x/auth/module.go b/x/auth/module.go index 32f5f7627b90..d9290c0a8f1f 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -27,7 +27,7 @@ import ( // ConsensusVersion defines the current x/auth module consensus version. const ( - ConsensusVersion = 6 + ConsensusVersion = 5 GovModuleName = "gov" ) @@ -113,9 +113,6 @@ func (am AppModule) RegisterMigrations(mr appmodule.MigrationRegistrar) error { if err := mr.Register(types.ModuleName, 4, m.Migrate4To5); err != nil { return fmt.Errorf("failed to migrate x/%s from version 4 to 5: %w", types.ModuleName, err) } - if err := mr.Register(types.ModuleName, 5, m.Migrate5To6); err != nil { - return fmt.Errorf("failed to migrate x/%s from version 5 to 6: %w", types.ModuleName, err) - } return nil } diff --git a/x/genutil/client/cli/init.go b/x/genutil/client/cli/init.go index 19c3d148c48f..866a472778e0 100644 --- a/x/genutil/client/cli/init.go +++ b/x/genutil/client/cli/init.go @@ -9,10 +9,14 @@ import ( "os" "path/filepath" - errorsmod "cosmossdk.io/errors" - "cosmossdk.io/math/unsafe" cfg "github.com/cometbft/cometbft/config" cmttypes "github.com/cometbft/cometbft/types" + "github.com/cosmos/go-bip39" + "github.com/spf13/cobra" + + errorsmod "cosmossdk.io/errors" + "cosmossdk.io/math/unsafe" + "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/client/input" @@ -21,8 +25,6 @@ import ( "github.com/cosmos/cosmos-sdk/version" "github.com/cosmos/cosmos-sdk/x/genutil" "github.com/cosmos/cosmos-sdk/x/genutil/types" - "github.com/cosmos/go-bip39" - "github.com/spf13/cobra" ) const (