fix to use amino multisig pubkey at migration (#560)

* fix to use amino multisig pubkey at migration * add upgrade handler for bombay-11 network softfork * add changelog * add LazyGradedVestingAccount multisig account migration test
terra-money · Sep 24, 2021 · 4f37d36 · 4f37d36
1 parent 9d1edcc
commit 4f37d36
Show file tree

Hide file tree

Showing 6 changed files with 179 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,8 @@
+## [v0.5.4]
+
+### Bug Fixes
+- [\#560](https://github.com/terra-money/core/pull/560) Fix migration bug of multisig pubkey which was in v040 auth module migration.
+
 ## [v0.5.3]
 
 ### Improvement

diff --git a/app/app.go b/app/app.go
@@ -98,6 +98,7 @@ import (
 	customauth "github.com/terra-money/core/custom/auth"
 	customante "github.com/terra-money/core/custom/auth/ante"
 	customauthrest "github.com/terra-money/core/custom/auth/client/rest"
+	customlegacyauthv040 "github.com/terra-money/core/custom/auth/legacy/v040"
 	customauthsim "github.com/terra-money/core/custom/auth/simulation"
 	customauthtx "github.com/terra-money/core/custom/auth/tx"
 	customauthz "github.com/terra-money/core/custom/authz"
@@ -349,12 +350,6 @@ func NewTerraApp(
 	app.FeeGrantKeeper = feegrantkeeper.NewKeeper(appCodec, keys[feegrant.StoreKey], app.AccountKeeper)
 	app.UpgradeKeeper = upgradekeeper.NewKeeper(skipUpgradeHeights, keys[upgradetypes.StoreKey], appCodec, homePath, app.BaseApp)
 
-	// clear SoftwareUpgradeProposal
-	app.UpgradeKeeper.SetUpgradeHandler("v0.5.0", func(ctx sdk.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) {
-		// Just pass current version map, no migration required
-		return app.mm.RunMigrations(ctx, app.configurator, app.mm.GetVersionMap())
-	})
-
 	// register the staking hooks
 	// NOTE: stakingKeeper above is passed by reference, so that it will contain these hooks
 	app.StakingKeeper = *stakingKeeper.SetHooks(
@@ -587,8 +582,37 @@ func NewTerraApp(
 // Name returns the name of the App
 func (app *TerraApp) Name() string { return app.BaseApp.Name() }
 
+// MultiSigPubKeyMigrationPlanName upgrade plan name to fix multisig pubkey migration problem
+const MultiSigPubKeyMigrationPlanName = "multisig-pubkey-migration"
+
+// MultiSigPubKeyMigrationChainID target chain-id to apply multisig pubkey migration fix
+const MultiSigPubKeyMigrationChainID = "bombay-11"
+
 // BeginBlocker application updates every begin block
 func (app *TerraApp) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock {
+
+	// NOTE: this handler is only for bombay-11, due to pubkey migration had problem
+	if ctx.ChainID() == MultiSigPubKeyMigrationChainID {
+		if plan, found := app.UpgradeKeeper.GetUpgradePlan(ctx); found {
+			if plan.Name == MultiSigPubKeyMigrationPlanName && plan.Height == ctx.BlockHeight() {
+				app.UpgradeKeeper.SetUpgradeHandler(MultiSigPubKeyMigrationPlanName, func(ctx sdk.Context, _ upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) {
+					var err error
+					app.AccountKeeper.IterateAccounts(ctx, func(account authtypes.AccountI) bool {
+						if account, err = customlegacyauthv040.MigrateAccount(account); err != nil {
+							return true
+						} else if account != nil {
+							app.AccountKeeper.SetAccount(ctx, account)
+						}
+
+						return false
+					})
+
+					return vm, err
+				})
+			}
+		}
+	}
+
 	return app.mm.BeginBlock(ctx, req)
 }
 

diff --git a/custom/auth/legacy/v039/types.go b/custom/auth/legacy/v039/types.go
@@ -7,12 +7,15 @@ import (
 	"errors"
 
 	"github.com/cosmos/cosmos-sdk/codec"
-	"github.com/cosmos/cosmos-sdk/codec/legacy"
-	cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
+	"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
+	kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
+	"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
 	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	v038auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v038"
 	v039auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v039"
+
+	"github.com/tendermint/tendermint/crypto/sr25519"
 )
 
 type (
@@ -56,8 +59,18 @@ type (
 
 	// VestingSchedules nolint
 	VestingSchedules []VestingSchedule
+
+	// LegacyAminoPubKey specifies a public key type
+	// which nests multiple public keys and a threshold,
+	// it uses legacy amino address rules.
+	LegacyAminoPubKey struct {
+		Threshold sdk.Int              `json:"threshold"`
+		PubKeys   []cryptotypes.PubKey `json:"pubkeys"`
+	}
 )
 
+var _ cryptotypes.PubKey = &LegacyAminoPubKey{}
+
 // NewLazyGradedVestingAccountRaw nolint
 func NewLazyGradedVestingAccountRaw(baseVestingAcc *v039auth.BaseVestingAccount, lazyVestingSchedules VestingSchedules) *LazyGradedVestingAccount {
 	return &LazyGradedVestingAccount{
@@ -135,13 +148,13 @@ func (lgva LazyGradedVestingAccount) MarshalJSON() ([]byte, error) {
 		VestingSchedules: lgva.VestingSchedules,
 	}
 
-	return legacy.Cdc.MarshalJSON(alias)
+	return internalCdc.MarshalJSON(alias)
 }
 
 // UnmarshalJSON unmarshals raw JSON bytes into a LazyGradedVestingAccount.
 func (lgva *LazyGradedVestingAccount) UnmarshalJSON(bz []byte) error {
 	var alias vestingAccountJSON
-	if err := legacy.Cdc.UnmarshalJSON(bz, &alias); err != nil {
+	if err := internalCdc.UnmarshalJSON(bz, &alias); err != nil {
 		return err
 	}
 
@@ -158,13 +171,70 @@ func (lgva *LazyGradedVestingAccount) UnmarshalJSON(bz []byte) error {
 	return nil
 }
 
+// Address implements cryptotypes.PubKey Address method
+func (*LegacyAminoPubKey) Address() cryptotypes.Address {
+	return nil
+}
+
+// Bytes no-lint
+func (*LegacyAminoPubKey) Bytes() []byte { return nil }
+
+// Equals no-lint
+func (*LegacyAminoPubKey) Equals(key cryptotypes.PubKey) bool { return false }
+
+// ProtoMessage no-lint
+func (*LegacyAminoPubKey) ProtoMessage() {}
+
+// Reset no-lint
+func (*LegacyAminoPubKey) Reset() {}
+
+// String no-lint
+func (*LegacyAminoPubKey) String() string { return "not implemented" }
+
+// Type no-lint
+func (*LegacyAminoPubKey) Type() string { return "PubKeyMultisigThreshold" }
+
+// VerifySignature no-lint
+func (*LegacyAminoPubKey) VerifySignature(msg []byte, sig []byte) bool {
+	panic("not implemented")
+}
+
+var internalCdc *codec.LegacyAmino
+
+func init() {
+	internalCdc = codec.NewLegacyAmino()
+	RegisterLegacyAminoCodec(internalCdc)
+}
+
 // RegisterLegacyAminoCodec nonlint
 func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) {
-	cryptocodec.RegisterCrypto(cdc)
+	registerCrypto(cdc)
 	cdc.RegisterInterface((*v038auth.GenesisAccount)(nil), nil)
 	cdc.RegisterInterface((*v038auth.Account)(nil), nil)
 	cdc.RegisterConcrete(&v039auth.BaseAccount{}, "core/Account", nil)
 	cdc.RegisterConcrete(&v039auth.BaseVestingAccount{}, "core/BaseVestingAccount", nil)
 	cdc.RegisterConcrete(&LazyGradedVestingAccount{}, "core/LazyGradedVestingAccount", nil)
 	cdc.RegisterConcrete(&v039auth.ModuleAccount{}, "supply/ModuleAccount", nil)
 }
+
+// registerCrypto registers all crypto dependency types with the provided Amino
+// codec.
+func registerCrypto(cdc *codec.LegacyAmino) {
+	cdc.RegisterInterface((*cryptotypes.PubKey)(nil), nil)
+	cdc.RegisterConcrete(sr25519.PubKey{},
+		sr25519.PubKeyName, nil)
+	cdc.RegisterConcrete(&ed25519.PubKey{},
+		ed25519.PubKeyName, nil)
+	cdc.RegisterConcrete(&secp256k1.PubKey{},
+		secp256k1.PubKeyName, nil)
+	cdc.RegisterConcrete(&LegacyAminoPubKey{},
+		kmultisig.PubKeyAminoRoute, nil)
+
+	cdc.RegisterInterface((*cryptotypes.PrivKey)(nil), nil)
+	cdc.RegisterConcrete(sr25519.PrivKey{},
+		sr25519.PrivKeyName, nil)
+	cdc.RegisterConcrete(&ed25519.PrivKey{}, //nolint:staticcheck
+		ed25519.PrivKeyName, nil)
+	cdc.RegisterConcrete(&secp256k1.PrivKey{},
+		secp256k1.PrivKeyName, nil)
+}
diff --git a/custom/auth/legacy/v040/migrate.go b/custom/auth/legacy/v040/migrate.go
@@ -2,6 +2,8 @@ package v040
 
 import (
 	codectypes "github.com/cosmos/cosmos-sdk/codec/types"
+	kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
+	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	v039auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v039"
@@ -17,11 +19,19 @@ import (
 // convertBaseAccount converts a 0.39 BaseAccount to a 0.40 BaseAccount.
 func convertBaseAccount(old *v039auth.BaseAccount) *v040auth.BaseAccount {
 	var any *codectypes.Any
+
 	// If the old genesis had a pubkey, we pack it inside an Any. Or else, we
 	// just leave it nil.
 	if old.PubKey != nil {
+		var pk cryptotypes.PubKey
+		if mpk, ok := old.PubKey.(*v039authcustom.LegacyAminoPubKey); ok {
+			pk = kmultisig.NewLegacyAminoPubKey(int(mpk.Threshold.Int64()), mpk.PubKeys)
+		} else {
+			pk = old.PubKey
+		}
+
 		var err error
-		any, err = codectypes.NewAnyWithValue(old.PubKey)
+		any, err = codectypes.NewAnyWithValue(pk)
 		if err != nil {
 			panic(err)
 		}

diff --git a/custom/auth/legacy/v040/migrate_test.go b/custom/auth/legacy/v040/migrate_test.go
@@ -0,0 +1,31 @@
+package v040
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	v039authcustom "github.com/terra-money/core/custom/auth/legacy/v039"
+
+	"github.com/cosmos/cosmos-sdk/codec"
+	"github.com/cosmos/cosmos-sdk/crypto/types/multisig"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	v038auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v038"
+)
+
+func TestMultisigPubkeyMigration(t *testing.T) {
+	config := sdk.GetConfig()
+	config.SetBech32PrefixForAccount("terra", "terrapub")
+
+	v04Codec := codec.NewLegacyAmino()
+	v039authcustom.RegisterLegacyAminoCodec(v04Codec)
+	jsonAccount := `{"type":"core/LazyGradedVestingAccount","value":{"address":"terra1dp0taj85ruc299rkdvzp4z5pfg6z6swaed74e6","coins":[],"public_key":{"type":"tendermint/PubKeyMultisigThreshold","value":{"threshold":"2","pubkeys":[{"type":"tendermint/PubKeySecp256k1","value":"AyETa9Y9ihObzeRPWMP0MBAa0Mqune3I+5KonOCPTtkv"},{"type":"tendermint/PubKeySecp256k1","value":"AzzLltyI4MzxLpcmS1vfpXJeAk/sgS1eVYmvXgFpGRtg"},{"type":"tendermint/PubKeySecp256k1","value":"AnZjvWmye3JPEL95xRcGeFRf4o8pHDK0dkZjf6B9D4FA"}]}},"account_number":"317776","sequence":"61","original_vesting":[{"denom":"usdr","amount":"1000000000000000"}],"delegated_free":[{"denom":"uluna","amount":"57620630000008"}],"delegated_vesting":[],"end_time":"0","vesting_schedules":[{"denom":"usdr","schedules":[{"start_time":"1556085600","end_time":"1558677600","ratio":"0.100000000000000000"},{"start_time":"1587708000","end_time":"1590300000","ratio":"0.100000000000000000"},{"start_time":"1619244000","end_time":"1621836000","ratio":"0.100000000000000000"},{"start_time":"1650780000","end_time":"1653372000","ratio":"0.100000000000000000"},{"start_time":"1682316000","end_time":"1684908000","ratio":"0.100000000000000000"},{"start_time":"1713938400","end_time":"1716530400","ratio":"0.100000000000000000"},{"start_time":"1745474400","end_time":"1748066400","ratio":"0.100000000000000000"},{"start_time":"1777010400","end_time":"1779602400","ratio":"0.100000000000000000"},{"start_time":"1808546400","end_time":"1811138400","ratio":"0.100000000000000000"},{"start_time":"1840168800","end_time":"1842760800","ratio":"0.100000000000000000"}]}]}}`
+
+	var oldAccount v038auth.GenesisAccount
+	v04Codec.MustUnmarshalJSON([]byte(jsonAccount), &oldAccount)
+
+	account := convertBaseVestingAccount(oldAccount.(*v039authcustom.LazyGradedVestingAccount).BaseVestingAccount)
+
+	pk, ok := account.GetPubKey().(multisig.PubKey)
+	require.True(t, ok)
+	require.Equal(t, 3, len(pk.GetPubKeys()))
+}
diff --git a/custom/auth/legacy/v040/store.go b/custom/auth/legacy/v040/store.go
@@ -0,0 +1,27 @@
+// Package v040 creates in-place store migrations for fixing
+// multisig pubkey migration problem
+// ref: https://github.com/terra-money/core/issues/562
+package v040
+
+import (
+	"github.com/cosmos/cosmos-sdk/crypto/types/multisig"
+	"github.com/cosmos/cosmos-sdk/x/auth/types"
+)
+
+// Set MultiSig account PubKey as nil
+func migrateMultiSigAccount(account types.AccountI) (types.AccountI, error) {
+	_, ok := account.GetPubKey().(multisig.PubKey)
+	if !ok {
+		return nil, nil
+	}
+
+	_ = account.SetPubKey(nil)
+	return account.(types.AccountI), nil
+}
+
+// MigrateAccount migrates multisig account's PubKey as nil to restore mistakenly set PubKey
+// References: https://github.com/terra-money/core/issues/562
+//
+func MigrateAccount(account types.AccountI) (types.AccountI, error) {
+	return migrateMultiSigAccount(account)
+}