refactor(auth): Remove IterateAccounts method from x/auth keeper (#19363

)
cosmos · Feb 6, 2024 · e604e54 · e604e54
1 parent 41b188d
commit e604e54
Show file tree

Hide file tree

Showing 11 changed files with 44 additions and 49 deletions.
diff --git a/tests/integration/gov/genesis_test.go b/tests/integration/gov/genesis_test.go
@@ -100,7 +100,8 @@ func TestImportExportQueues(t *testing.T) {
 	assert.Assert(t, proposal1.Status == v1.StatusDepositPeriod)
 	assert.Assert(t, proposal2.Status == v1.StatusVotingPeriod)
 
-	authGenState := s1.AccountKeeper.ExportGenesis(ctx)
+	authGenState, err := s1.AccountKeeper.ExportGenesis(ctx)
+	require.NoError(t, err)
 	bankGenState := s1.BankKeeper.ExportGenesis(ctx)
 	stakingGenState := s1.StakingKeeper.ExportGenesis(ctx)
 

diff --git a/x/auth/CHANGELOG.md b/x/auth/CHANGELOG.md
@@ -43,6 +43,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 * [#17985](https://github.com/cosmos/cosmos-sdk/pull/17985) Remove `StdTxConfig`
 * [#19161](https://github.com/cosmos/cosmos-sdk/pull/19161) Remove `simulate` from `SetGasMeter`
+* [#19363](https://github.com/cosmos/cosmos-sdk/pull/19363) Remove `IterateAccounts` and `GetAllAccounts` methods from the AccountKeeper interface and Keeper.
 
 ### Consensus Breaking Changes
 

diff --git a/x/auth/keeper/account.go b/x/auth/keeper/account.go
@@ -44,16 +44,6 @@ func (ak AccountKeeper) GetAccount(ctx context.Context, addr sdk.AccAddress) sdk
 	return acc
 }
 
-// GetAllAccounts returns all accounts in the accountKeeper.
-func (ak AccountKeeper) GetAllAccounts(ctx context.Context) (accounts []sdk.AccountI) {
-	ak.IterateAccounts(ctx, func(acc sdk.AccountI) (stop bool) {
-		accounts = append(accounts, acc)
-		return false
-	})
-
-	return accounts
-}
-
 // SetAccount implements AccountKeeperI.
 func (ak AccountKeeper) SetAccount(ctx context.Context, acc sdk.AccountI) {
 	err := ak.Accounts.Set(ctx, acc.GetAddress(), acc)
@@ -70,14 +60,3 @@ func (ak AccountKeeper) RemoveAccount(ctx context.Context, acc sdk.AccountI) {
 		panic(err)
 	}
 }
-
-// IterateAccounts iterates over all the stored accounts and performs a callback function.
-// Stops iteration when callback returns true.
-func (ak AccountKeeper) IterateAccounts(ctx context.Context, cb func(account sdk.AccountI) (stop bool)) {
-	err := ak.Accounts.Walk(ctx, nil, func(_ sdk.AccAddress, value sdk.AccountI) (bool, error) {
-		return cb(value), nil
-	})
-	if err != nil {
-		panic(err)
-	}
-}
diff --git a/x/auth/keeper/genesis.go b/x/auth/keeper/genesis.go
@@ -2,6 +2,7 @@ package keeper
 
 import (
 	"context"
+	"fmt"
 
 	"cosmossdk.io/x/auth/types"
 
@@ -12,14 +13,14 @@ import (
 //
 // CONTRACT: old coins from the FeeCollectionKeeper need to be transferred through
 // a genesis port script to the new fee collector account
-func (ak AccountKeeper) InitGenesis(ctx context.Context, data types.GenesisState) {
+func (ak AccountKeeper) InitGenesis(ctx context.Context, data types.GenesisState) error {
 	if err := ak.Params.Set(ctx, data.Params); err != nil {
-		panic(err)
+		return err
 	}
 
 	accounts, err := types.UnpackAccounts(data.Accounts)
 	if err != nil {
-		panic(err)
+		return err
 	}
 	accounts = types.SanitizeGenesisAccounts(accounts)
 
@@ -35,18 +36,21 @@ func (ak AccountKeeper) InitGenesis(ctx context.Context, data types.GenesisState
 	}
 
 	ak.GetModuleAccount(ctx, types.FeeCollectorName)
+	return nil
 }
 
 // ExportGenesis returns a GenesisState for a given context and keeper
-func (ak AccountKeeper) ExportGenesis(ctx context.Context) *types.GenesisState {
+func (ak AccountKeeper) ExportGenesis(ctx context.Context) (*types.GenesisState, error) {
 	params := ak.GetParams(ctx)
 
 	var genAccounts types.GenesisAccounts
-	ak.IterateAccounts(ctx, func(account sdk.AccountI) bool {
-		genAccount := account.(types.GenesisAccount)
-		genAccounts = append(genAccounts, genAccount)
-		return false
+	err := ak.Accounts.Walk(ctx, nil, func(key sdk.AccAddress, value sdk.AccountI) (stop bool, err error) {
+		genAcc, ok := value.(types.GenesisAccount)
+		if !ok {
+			return true, fmt.Errorf("unable to conver account with address %s into a genesis account: type %T", key, value)
+		}
+		genAccounts = append(genAccounts, genAcc)
+		return false, nil
 	})
-
-	return types.NewGenesisState(params, genAccounts)
+	return types.NewGenesisState(params, genAccounts), err
 }
diff --git a/x/auth/keeper/keeper.go b/x/auth/keeper/keeper.go
@@ -39,9 +39,6 @@ type AccountKeeperI interface {
 	// Remove an account from the store.
 	RemoveAccount(context.Context, sdk.AccountI)
 
-	// Iterate over all accounts, calling the provided function. Stop iteration when it returns true.
-	IterateAccounts(context.Context, func(sdk.AccountI) bool)
-
 	// Fetch the public key of an account at a specified address
 	GetPubKey(context.Context, sdk.AccAddress) (cryptotypes.PubKey, error)
 

diff --git a/x/auth/keeper/keeper_test.go b/x/auth/keeper/keeper_test.go
@@ -3,6 +3,7 @@ package keeper_test
 import (
 	"testing"
 
+	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 
 	"cosmossdk.io/core/header"
@@ -107,7 +108,8 @@ func (suite *KeeperTestSuite) TestInitGenesis() {
 	}
 
 	ctx := suite.ctx
-	suite.accountKeeper.InitGenesis(ctx, genState)
+	err := suite.accountKeeper.InitGenesis(ctx, genState)
+	require.NoError(suite.T(), err)
 
 	params := suite.accountKeeper.GetParams(ctx)
 	suite.Require().Equal(genState.Params.MaxMemoCharacters, params.MaxMemoCharacters, "MaxMemoCharacters")
@@ -153,9 +155,15 @@ func (suite *KeeperTestSuite) TestInitGenesis() {
 		genState.Accounts = append(genState.Accounts, codectypes.UnsafePackAny(acct))
 	}
 
-	suite.accountKeeper.InitGenesis(ctx, genState)
+	err = suite.accountKeeper.InitGenesis(ctx, genState)
+	require.NoError(suite.T(), err)
 
-	keeperAccts := suite.accountKeeper.GetAllAccounts(ctx)
+	var keeperAccts []sdk.AccountI
+	err = suite.accountKeeper.Accounts.Walk(ctx, nil, func(_ sdk.AccAddress, value sdk.AccountI) (stop bool, err error) {
+		keeperAccts = append(keeperAccts, value)
+		return false, nil
+	})
+	require.NoError(suite.T(), err)
 	// len(accts)+1 because we initialize fee_collector account after the genState accounts
 	suite.Require().Equal(len(keeperAccts), len(accts)+1, "number of accounts in the keeper vs in genesis state")
 	for i, genAcct := range accts {
@@ -200,9 +208,15 @@ func (suite *KeeperTestSuite) TestInitGenesis() {
 		},
 	}
 
-	suite.accountKeeper.InitGenesis(ctx, genState)
+	err = suite.accountKeeper.InitGenesis(ctx, genState)
+	require.NoError(suite.T(), err)
 
-	keeperAccts = suite.accountKeeper.GetAllAccounts(ctx)
+	keeperAccts = nil
+	err = suite.accountKeeper.Accounts.Walk(ctx, nil, func(_ sdk.AccAddress, value sdk.AccountI) (stop bool, err error) {
+		keeperAccts = append(keeperAccts, value)
+		return false, nil
+	})
+	require.NoError(suite.T(), err)
 	// len(genState.Accounts)+1 because we initialize fee_collector as account number 1 (last)
 	suite.Require().Equal(len(keeperAccts), len(genState.Accounts)+1, "number of accounts in the keeper vs in genesis state")
 

diff --git a/x/auth/module.go b/x/auth/module.go
@@ -126,13 +126,19 @@ func (am AppModule) RegisterServices(cfg module.Configurator) {
 func (am AppModule) InitGenesis(ctx context.Context, cdc codec.JSONCodec, data json.RawMessage) {
 	var genesisState types.GenesisState
 	cdc.MustUnmarshalJSON(data, &genesisState)
-	am.accountKeeper.InitGenesis(ctx, genesisState)
+	err := am.accountKeeper.InitGenesis(ctx, genesisState)
+	if err != nil {
+		panic(err)
+	}
 }
 
 // ExportGenesis returns the exported genesis state as raw bytes for the auth
 // module.
 func (am AppModule) ExportGenesis(ctx context.Context, cdc codec.JSONCodec) json.RawMessage {
-	gs := am.accountKeeper.ExportGenesis(ctx)
+	gs, err := am.accountKeeper.ExportGenesis(ctx)
+	if err != nil {
+		panic(err)
+	}
 	return cdc.MustMarshalJSON(gs)
 }
 

diff --git a/x/bank/types/expected_keepers.go b/x/bank/types/expected_keepers.go
@@ -18,12 +18,8 @@ type AccountKeeper interface {
 	NewAccountWithAddress(ctx context.Context, addr sdk.AccAddress) sdk.AccountI
 
 	GetAccount(ctx context.Context, addr sdk.AccAddress) sdk.AccountI
-	GetAllAccounts(ctx context.Context) []sdk.AccountI
 	HasAccount(ctx context.Context, addr sdk.AccAddress) bool
 	SetAccount(ctx context.Context, acc sdk.AccountI)
-
-	IterateAccounts(ctx context.Context, process func(sdk.AccountI) bool)
-
 	ValidatePermissions(macc sdk.ModuleAccountI) error
 
 	GetModuleAddress(moduleName string) sdk.AccAddress

diff --git a/x/genutil/types/expected_keepers.go b/x/genutil/types/expected_keepers.go
@@ -21,7 +21,6 @@ type StakingKeeper interface {
 type AccountKeeper interface {
 	NewAccount(context.Context, sdk.AccountI) sdk.AccountI
 	SetAccount(context.Context, sdk.AccountI)
-	IterateAccounts(ctx context.Context, process func(sdk.AccountI) (stop bool))
 }
 
 // GenesisAccountsIterator defines the expected iterating genesis accounts object (noalias)

diff --git a/x/slashing/types/expected_keepers.go b/x/slashing/types/expected_keepers.go
@@ -15,7 +15,6 @@ import (
 type AccountKeeper interface {
 	AddressCodec() address.Codec
 	GetAccount(ctx context.Context, addr sdk.AccAddress) sdk.AccountI
-	IterateAccounts(ctx context.Context, process func(sdk.AccountI) (stop bool))
 }
 
 // BankKeeper defines the expected interface needed to retrieve account balances.

diff --git a/x/staking/types/expected_keepers.go b/x/staking/types/expected_keepers.go
@@ -17,7 +17,6 @@ import (
 type AccountKeeper interface {
 	AddressCodec() address.Codec
 
-	IterateAccounts(ctx context.Context, process func(sdk.AccountI) (stop bool))
 	GetAccount(ctx context.Context, addr sdk.AccAddress) sdk.AccountI // only used for simulation
 
 	GetModuleAddress(name string) sdk.AccAddress