Skip to content
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

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

Merged
merged 3 commits into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 2 additions & 1 deletion tests/integration/gov/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
21 changes: 0 additions & 21 deletions x/auth/keeper/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
}
24 changes: 14 additions & 10 deletions x/auth/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keeper

import (
"context"
"fmt"

"cosmossdk.io/x/auth/types"

Expand All @@ -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)

Expand All @@ -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
}
3 changes: 0 additions & 3 deletions x/auth/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
24 changes: 19 additions & 5 deletions x/auth/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package keeper_test
import (
"testing"

"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"

"cosmossdk.io/core/header"
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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")

Expand Down
10 changes: 8 additions & 2 deletions x/auth/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
4 changes: 0 additions & 4 deletions x/bank/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion x/genutil/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion x/staking/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading