Fix auth genesis account number handling.

CHANGELOG.md

@@ -39,7 +39,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## Unreleased
 
-* nothing
+### Bug Fixes
+
+* (x/auth) Add the missing account number case to the sim state decoder. TODO: link PR.
+* (x/auth) Handle missing account numbers during InitGenesis. TODO: link PR.
 
 ---
 

x/auth/keeper/genesis.go

@@ -18,8 +18,14 @@ func (ak AccountKeeper) InitGenesis(ctx sdk.Context, data types.GenesisState) {
 	}
 	accounts = types.SanitizeGenesisAccounts(accounts)
 
-	for _, a := range accounts {
-		acc := ak.NewAccount(ctx, a)
+	// Set the accounts and make sure the global account number matches the largest account number (even if zero).
+	var lastAccNum *uint64
+	for _, acc := range accounts {
+		accNum := acc.GetAccountNumber()
+		for lastAccNum == nil || *lastAccNum < accNum {
+			n := ak.GetNextAccountNumber(ctx)
+			lastAccNum = &n
+		}
 		ak.SetAccount(ctx, acc)
 	}
 

x/auth/keeper/keeper_test.go

@@ -3,14 +3,26 @@ package keeper_test
 import (
 	"testing"
 
+	massert "github.com/magiconair/properties/assert"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 
+	"github.com/tendermint/tendermint/libs/log"
+	tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
+
 	"github.com/cosmos/cosmos-sdk/baseapp"
+	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
+	"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
 	"github.com/cosmos/cosmos-sdk/simapp"
+	"github.com/cosmos/cosmos-sdk/store"
+	storetypes "github.com/cosmos/cosmos-sdk/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/auth/keeper"
 	"github.com/cosmos/cosmos-sdk/x/auth/types"
+	paramskeeper "github.com/cosmos/cosmos-sdk/x/params/keeper"
+	paramstypes "github.com/cosmos/cosmos-sdk/x/params/types"
+	dbm "github.com/tendermint/tm-db"
 )
 
 const (
@@ -146,3 +158,144 @@ func TestSupply_ValidatePermissions(t *testing.T) {
 	err = app.AccountKeeper.ValidatePermissions(otherAcc)
 	require.Error(t, err)
 }
+
+func TestInitGenesis(tt *testing.T) {
+	authKey := sdk.NewKVStoreKey(types.StoreKey)
+	paramsKey := sdk.NewKVStoreKey(paramstypes.StoreKey)
+	paramsTKey := sdk.NewTransientStoreKey(paramstypes.TStoreKey)
+	newCtx := func(t *testing.T) sdk.Context {
+		db := dbm.NewMemDB()
+		cms := store.NewCommitMultiStore(db)
+		cms.MountStoreWithDB(authKey, storetypes.StoreTypeIAVL, db)
+		cms.MountStoreWithDB(paramsKey, storetypes.StoreTypeIAVL, db)
+		cms.MountStoreWithDB(paramsTKey, storetypes.StoreTypeTransient, db)
+		err := cms.LoadLatestVersion()
+		if err != nil {
+			panic(err)
+		}
+		return sdk.NewContext(cms, tmproto.Header{}, false, log.NewNopLogger())
+	}
+	newAccountKeeper := func(t *testing.T) keeper.AccountKeeper {
+		encConf := simapp.MakeTestEncodingConfig()
+		paramsKeeper := paramskeeper.NewKeeper(encConf.Codec, encConf.Amino, paramsKey, paramsTKey)
+		return keeper.NewAccountKeeper(
+			encConf.Codec, authKey, paramsKeeper.Subspace(types.ModuleName),
+			types.ProtoBaseAccount, map[string][]string{}, sdk.Bech32MainPrefix,
+		)
+	}
+
+	tt.Run("params are set", func(t *testing.T) {
+		genState := types.GenesisState{
+			Params: types.Params{
+				MaxMemoCharacters:      types.DefaultMaxMemoCharacters + 1,
+				TxSigLimit:             types.DefaultTxSigLimit + 1,
+				TxSizeCostPerByte:      types.DefaultTxSizeCostPerByte + 1,
+				SigVerifyCostED25519:   types.DefaultSigVerifyCostED25519 + 1,
+				SigVerifyCostSecp256k1: types.DefaultSigVerifyCostSecp256k1 + 1,
+			},
+			Accounts: []*codectypes.Any{},
+		}
+
+		accKeeper := newAccountKeeper(t)
+		ctx := newCtx(t)
+
+		accKeeper.InitGenesis(ctx, genState)
+
+		params := accKeeper.GetParams(ctx)
+		massert.Equal(t, genState.Params.MaxMemoCharacters, params.MaxMemoCharacters, "MaxMemoCharacters")
+		massert.Equal(t, genState.Params.TxSigLimit, params.TxSigLimit, "TxSigLimit")
+		massert.Equal(t, genState.Params.TxSizeCostPerByte, params.TxSizeCostPerByte, "TxSizeCostPerByte")
+		massert.Equal(t, genState.Params.SigVerifyCostED25519, params.SigVerifyCostED25519, "SigVerifyCostED25519")
+		massert.Equal(t, genState.Params.SigVerifyCostSecp256k1, params.SigVerifyCostSecp256k1, "SigVerifyCostSecp256k1")
+	})
+
+	tt.Run("duplicate account numbers are fixed", func(t *testing.T) {
+		pubKey1 := ed25519.GenPrivKey().PubKey()
+		pubKey2 := ed25519.GenPrivKey().PubKey()
+		accts := []types.AccountI{
+			&types.BaseAccount{
+				Address:       sdk.AccAddress(pubKey1.Address()).String(),
+				PubKey:        codectypes.UnsafePackAny(pubKey1),
+				AccountNumber: 0,
+				Sequence:      5,
+			},
+			&types.ModuleAccount{
+				BaseAccount: &types.BaseAccount{
+					Address:       types.NewModuleAddress("testing").String(),
+					PubKey:        nil,
+					AccountNumber: 0,
+					Sequence:      6,
+				},
+				Name:        "testing",
+				Permissions: nil,
+			},
+			&types.BaseAccount{
+				Address:       sdk.AccAddress(pubKey2.Address()).String(),
+				PubKey:        codectypes.UnsafePackAny(pubKey2),
+				AccountNumber: 5,
+				Sequence:      7,
+			},
+		}
+		genState := types.GenesisState{
+			Params:   types.DefaultParams(),
+			Accounts: nil,
+		}
+		for _, acct := range accts {
+			genState.Accounts = append(genState.Accounts, codectypes.UnsafePackAny(acct))
+		}
+
+		accKeeper := newAccountKeeper(t)
+		ctx := newCtx(t)
+
+		accKeeper.InitGenesis(ctx, genState)
+
+		keeperAccts := accKeeper.GetAllAccounts(ctx)
+		assert.GreaterOrEqual(t, len(keeperAccts), len(accts), "number of accounts in the keeper vs in genesis state")
+		for i, genAcct := range accts {
+			genAcctAddr := genAcct.GetAddress()
+			var keeperAcct types.AccountI
+			for _, kacct := range keeperAccts {
+				if genAcctAddr.Equals(kacct.GetAddress()) {
+					keeperAcct = kacct
+					break
+				}
+			}
+			if assert.NotNilf(t, keeperAcct, "genesis account %s not in keeper accounts", genAcctAddr) {
+				assert.Equal(t, genAcct.GetPubKey(), keeperAcct.GetPubKey())
+				assert.Equal(t, genAcct.GetSequence(), keeperAcct.GetSequence())
+				if i == 1 {
+					assert.Equal(t, 1, int(keeperAcct.GetAccountNumber()))
+				} else {
+					assert.Equal(t, genAcct.GetSequence(), keeperAcct.GetSequence())
+				}
+			}
+		}
+
+		// The 3rd account has account number 5, so the next should be 6.
+		nextNum := accKeeper.GetNextAccountNumber(ctx)
+		assert.Equal(t, 6, int(nextNum))
+	})
+
+	tt.Run("one zero account still sets global account number", func(t *testing.T) {
+		pubKey1 := ed25519.GenPrivKey().PubKey()
+		genState := types.GenesisState{
+			Params: types.DefaultParams(),
+			Accounts: []*codectypes.Any{
+				codectypes.UnsafePackAny(&types.BaseAccount{
+					Address:       sdk.AccAddress(pubKey1.Address()).String(),
+					PubKey:        codectypes.UnsafePackAny(pubKey1),
+					AccountNumber: 0,
+					Sequence:      5,
+				}),
+			},
+		}
+
+		accKeeper := newAccountKeeper(t)
+		ctx := newCtx(t)
+
+		accKeeper.InitGenesis(ctx, genState)
+
+		nextNum := accKeeper.GetNextAccountNumber(ctx)
+		assert.Equal(t, 1, int(nextNum))
+	})
+}

x/auth/simulation/decoder.go

@@ -7,6 +7,7 @@ import (
 	gogotypes "github.com/gogo/protobuf/types"
 
 	"github.com/cosmos/cosmos-sdk/codec"
+	sdktypes "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/kv"
 	"github.com/cosmos/cosmos-sdk/x/auth/types"
 )
@@ -41,6 +42,20 @@ func NewDecodeStore(ak AuthUnmarshaler) func(kvA, kvB kv.Pair) string {
 
 			return fmt.Sprintf("GlobalAccNumberA: %d\nGlobalAccNumberB: %d", globalAccNumberA, globalAccNumberB)
 
+		case bytes.Equal(kvA.Key[:len(types.AccountNumberStoreKeyPrefix)], types.AccountNumberStoreKeyPrefix):
+			var accNumA, accNumB sdktypes.AccAddress
+			err := accNumA.Unmarshal(kvA.Value)
+			if err != nil {
+				panic(err)
+			}
+
+			err = accNumB.Unmarshal(kvB.Value)
+			if err != nil {
+				panic(err)
+			}
+
+			return fmt.Sprintf("AccNumA: %s\nAccNumB: %s", accNumA, accNumB)
+
 		default:
 			panic(fmt.Sprintf("unexpected %s key %X (%s)", types.ModuleName, kvA.Key, kvA.Key))
 		}

x/auth/simulation/decoder_test.go

@@ -41,6 +41,10 @@ func TestDecodeStore(t *testing.T) {
 				Key:   types.GlobalAccountNumberKey,
 				Value: cdc.MustMarshal(&globalAccNumber),
 			},
+			{
+				Key:   types.AccountNumberStoreKey(5),
+				Value: acc.GetAddress().Bytes(),
+			},
 			{
 				Key:   []byte{0x99},
 				Value: []byte{0x99},
@@ -53,6 +57,7 @@ func TestDecodeStore(t *testing.T) {
 	}{
 		{"Account", fmt.Sprintf("%v\n%v", acc, acc)},
 		{"GlobalAccNumber", fmt.Sprintf("GlobalAccNumberA: %d\nGlobalAccNumberB: %d", globalAccNumber, globalAccNumber)},
+		{"AccNum", fmt.Sprintf("AccNumA: %s\nAccNumB: %s", acc.GetAddress(), acc.GetAddress())},
 		{"other", ""},
 	}
 

x/auth/types/genesis.go

@@ -75,6 +75,43 @@ func ValidateGenesis(data GenesisState) error {
 
 // SanitizeGenesisAccounts sorts accounts and coin sets.
 func SanitizeGenesisAccounts(genAccs GenesisAccounts) GenesisAccounts {
+	// Make sure there aren't any duplicated account numbers by fixing the duplicates with the lowest unused values.
+	// seenAccNum = easy lookup for used account numbers.
+	seenAccNum := map[uint64]bool{}
+	// dupAccNum = a map of account number to accounts with duplicate account numbers (excluding the 1st one seen).
+	dupAccNum := map[uint64]GenesisAccounts{}
+	for _, acc := range genAccs {
+		num := acc.GetAccountNumber()
+		if !seenAccNum[num] {
+			seenAccNum[num] = true
+		} else {
+			dupAccNum[num] = append(dupAccNum[num], acc)
+		}
+	}
+	// dupAccNums a sorted list of the account numbers with duplicates.
+	var dupAccNums []uint64
+	for num := range dupAccNum {
+		dupAccNums = append(dupAccNums, num)
+	}
+	sort.Slice(dupAccNums, func(i, j int) bool {
+		return dupAccNums[i] < dupAccNums[j]
+	})
+	// Change the account number of the duplicated ones to the first unused value.
+	globalNum := uint64(0)
+	for _, dupNum := range dupAccNums {
+		accs := dupAccNum[dupNum]
+		for _, acc := range accs {
+			for seenAccNum[globalNum] {
+				globalNum++
+			}
+			if err := acc.SetAccountNumber(globalNum); err != nil {
+				panic(err)
+			}
+			seenAccNum[globalNum] = true
+		}
+	}
+
+	// Then sort them all by account number.
 	sort.Slice(genAccs, func(i, j int) bool {
 		return genAccs[i].GetAccountNumber() < genAccs[j].GetAccountNumber()
 	})

x/auth/types/genesis_test.go

@@ -14,22 +14,41 @@ import (
 )
 
 func TestSanitize(t *testing.T) {
-	addr1 := sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address())
-	authAcc1 := types.NewBaseAccountWithAddress(addr1)
-	err := authAcc1.SetAccountNumber(1)
-	require.NoError(t, err)
-
-	addr2 := sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address())
-	authAcc2 := types.NewBaseAccountWithAddress(addr2)
-
-	genAccs := types.GenesisAccounts{authAcc1, authAcc2}
-
-	require.True(t, genAccs[0].GetAccountNumber() > genAccs[1].GetAccountNumber())
-	require.Equal(t, genAccs[1].GetAddress(), addr2)
-	genAccs = types.SanitizeGenesisAccounts(genAccs)
-
-	require.False(t, genAccs[0].GetAccountNumber() > genAccs[1].GetAccountNumber())
-	require.Equal(t, genAccs[1].GetAddress(), addr1)
+	acc1Addr := sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address())
+	acc2Addr := sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address())
+
+	t.Run("accounts are sorted by account number", func(t *testing.T) {
+		acc1 := types.NewBaseAccountWithAddress(acc1Addr)
+		acc1.AccountNumber = 2
+		acc2 := types.NewBaseAccountWithAddress(acc2Addr)
+		acc2.AccountNumber = 0
+		acc3 := types.NewEmptyModuleAccount("testing")
+		acc3.BaseAccount.AccountNumber = 1
+
+		input := types.GenesisAccounts{acc1, acc2, acc3}
+		expected := types.GenesisAccounts{acc2, acc3, acc1}
+		actual := types.SanitizeGenesisAccounts(input)
+
+		require.Equal(t, expected, actual)
+	})
+
+	t.Run("duplicate account numbers are corrected", func(t *testing.T) {
+		acc1 := types.NewBaseAccountWithAddress(acc1Addr)
+		acc1.AccountNumber = 0
+		acc2 := types.NewBaseAccountWithAddress(acc2Addr)
+		acc2.AccountNumber = 1
+		acc3 := types.NewEmptyModuleAccount("testing")
+		acc3.BaseAccount.AccountNumber = 0
+
+		expAcc3 := types.NewEmptyModuleAccount("testing")
+		expAcc3.BaseAccount.AccountNumber = 2
+
+		input := types.GenesisAccounts{acc1, acc2, acc3}
+		expected := types.GenesisAccounts{acc1, acc2, expAcc3}
+		actual := types.SanitizeGenesisAccounts(input)
+
+		require.Equal(t, expected, actual)
+	})
 }
 
 var (