From d95b34e52739ff8da6d7fd9e3ed68181fc280b26 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Tue, 9 Mar 2021 14:15:08 +0100 Subject: [PATCH 01/13] Use v034auth RegisterCrypto --- x/auth/legacy/v034/types.go | 34 ++++++++++++++++++++++++++++++++ x/auth/legacy/v036/types.go | 9 ++++++++- x/auth/legacy/v038/types.go | 3 +-- x/auth/legacy/v039/types.go | 3 +-- x/genutil/legacy/v036/migrate.go | 5 ++--- x/genutil/legacy/v038/migrate.go | 1 + 6 files changed, 47 insertions(+), 8 deletions(-) diff --git a/x/auth/legacy/v034/types.go b/x/auth/legacy/v034/types.go index 83fa2d36eac..70d040ff4db 100644 --- a/x/auth/legacy/v034/types.go +++ b/x/auth/legacy/v034/types.go @@ -2,7 +2,13 @@ package v034 import ( + "github.com/cosmos/cosmos-sdk/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" + "github.com/tendermint/tendermint/crypto" ) const ( @@ -23,3 +29,31 @@ type ( Params Params `json:"params"` } ) + +// PubKeyMultisigThreshold implements a K of N threshold multisig. +// This struct is copy-pasted from: +// https://github.com/tendermint/tendermint/blob/v0.33.9/crypto/multisig/threshold_pubkey.go +type PubKeyMultisigThreshold struct { + K uint `json:"threshold"` + PubKeys []crypto.PubKey `json:"pubkeys"` +} + +func RegisterCrypto(cdc *codec.LegacyAmino) { + cdc.RegisterInterface((*cryptotypes.PubKey)(nil), nil) + cdc.RegisterConcrete(&ed25519.PubKey{}, + ed25519.PubKeyName, nil) + cdc.RegisterConcrete(&secp256k1.PubKey{}, + secp256k1.PubKeyName, nil) + cdc.RegisterConcrete(&PubKeyMultisigThreshold{}, + kmultisig.PubKeyAminoRoute, nil) + + cdc.RegisterInterface((*cryptotypes.PrivKey)(nil), nil) + cdc.RegisterConcrete(&ed25519.PrivKey{}, //nolint:staticcheck + ed25519.PrivKeyName, nil) + cdc.RegisterConcrete(&secp256k1.PrivKey{}, + secp256k1.PrivKeyName, nil) +} + +func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { + RegisterCrypto(cdc) +} diff --git a/x/auth/legacy/v036/types.go b/x/auth/legacy/v036/types.go index 908165f2650..dcef8779cd7 100644 --- a/x/auth/legacy/v036/types.go +++ b/x/auth/legacy/v036/types.go @@ -1,7 +1,10 @@ // DONTCOVER package v036 -import v034auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v034" +import ( + "github.com/cosmos/cosmos-sdk/codec" + v034auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v034" +) const ( ModuleName = "auth" @@ -16,3 +19,7 @@ type ( func NewGenesisState(params v034auth.Params) GenesisState { return GenesisState{params} } + +func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { + v034auth.RegisterCrypto(cdc) +} diff --git a/x/auth/legacy/v038/types.go b/x/auth/legacy/v038/types.go index b7dedf003e1..aec415bc7ce 100644 --- a/x/auth/legacy/v038/types.go +++ b/x/auth/legacy/v038/types.go @@ -13,7 +13,6 @@ import ( tmcrypto "github.com/tendermint/tendermint/crypto" "github.com/cosmos/cosmos-sdk/codec" - cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" v034auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v034" @@ -518,7 +517,7 @@ func ValidateGenAccounts(genAccounts GenesisAccounts) error { } func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { - cryptocodec.RegisterCrypto(cdc) + v034auth.RegisterCrypto(cdc) cdc.RegisterInterface((*GenesisAccount)(nil), nil) cdc.RegisterInterface((*Account)(nil), nil) cdc.RegisterConcrete(&BaseAccount{}, "cosmos-sdk/Account", nil) diff --git a/x/auth/legacy/v039/types.go b/x/auth/legacy/v039/types.go index 3de576f27ff..fbab9cf39fe 100644 --- a/x/auth/legacy/v039/types.go +++ b/x/auth/legacy/v039/types.go @@ -12,7 +12,6 @@ import ( "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/codec/legacy" - cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" v034auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v034" @@ -416,7 +415,7 @@ func (ma *ModuleAccount) UnmarshalJSON(bz []byte) error { } func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { - cryptocodec.RegisterCrypto(cdc) + v034auth.RegisterCrypto(cdc) cdc.RegisterInterface((*v038auth.GenesisAccount)(nil), nil) cdc.RegisterInterface((*v038auth.Account)(nil), nil) cdc.RegisterConcrete(&BaseAccount{}, "cosmos-sdk/Account", nil) diff --git a/x/genutil/legacy/v036/migrate.go b/x/genutil/legacy/v036/migrate.go index 204eebc233e..204b1b6a39a 100644 --- a/x/genutil/legacy/v036/migrate.go +++ b/x/genutil/legacy/v036/migrate.go @@ -3,7 +3,6 @@ package v036 import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" - cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" v034auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v034" v036auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v036" v036bank "github.com/cosmos/cosmos-sdk/x/bank/legacy/v036" @@ -22,11 +21,11 @@ import ( // Migrate migrates exported state from v0.34 to a v0.36 genesis state. func Migrate(appState types.AppMap, _ client.Context) types.AppMap { v034Codec := codec.NewLegacyAmino() - cryptocodec.RegisterCrypto(v034Codec) + v034auth.RegisterLegacyAminoCodec(v034Codec) v034gov.RegisterLegacyAminoCodec(v034Codec) v036Codec := codec.NewLegacyAmino() - cryptocodec.RegisterCrypto(v036Codec) + v036auth.RegisterLegacyAminoCodec(v036Codec) v036gov.RegisterLegacyAminoCodec(v036Codec) v036distr.RegisterLegacyAminoCodec(v036Codec) v036params.RegisterLegacyAminoCodec(v036Codec) diff --git a/x/genutil/legacy/v038/migrate.go b/x/genutil/legacy/v038/migrate.go index d0ca4c4186b..74240787ab9 100644 --- a/x/genutil/legacy/v038/migrate.go +++ b/x/genutil/legacy/v038/migrate.go @@ -19,6 +19,7 @@ import ( // Migrate migrates exported state from v0.36/v0.37 to a v0.38 genesis state. func Migrate(appState types.AppMap, _ client.Context) types.AppMap { v036Codec := codec.NewLegacyAmino() + v036auth.RegisterLegacyAminoCodec(v036Codec) v036gov.RegisterLegacyAminoCodec(v036Codec) v036distr.RegisterLegacyAminoCodec(v036Codec) v036params.RegisterLegacyAminoCodec(v036Codec) From d12f6ae25d658ddbcc0e3e81a369a72198083e9f Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 10 Mar 2021 14:54:32 +0100 Subject: [PATCH 02/13] Add custom amino for LegacyAminoPubKey --- crypto/keys/multisig/amino.go | 96 +++++++++++++++++++++++++++ crypto/keys/multisig/multisig_test.go | 78 +++++++++++++++++++++- 2 files changed, 172 insertions(+), 2 deletions(-) create mode 100644 crypto/keys/multisig/amino.go diff --git a/crypto/keys/multisig/amino.go b/crypto/keys/multisig/amino.go new file mode 100644 index 00000000000..d663df108ca --- /dev/null +++ b/crypto/keys/multisig/amino.go @@ -0,0 +1,96 @@ +package multisig + +import ( + types "github.com/cosmos/cosmos-sdk/codec/types" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" +) + +// tmMultisig implements a K of N threshold multisig. It is used for +// Amino marshaling of LegacyAminoPubKey (see below for details). +// +// This struct is copy-pasted from: +// https://github.com/tendermint/tendermint/blob/v0.33.9/crypto/multisig/threshold_pubkey.go +// +// This struct was used in the SDK <=0.39. In 0.40 and the switch to protobuf, +// it has been converted to LegacyAminoPubKey. However, there's one difference: +// the threshold field was an `uint` before, and an `uint32` after. This caused +// amino marshaling to be breaking: amino marshals `uint32` as a JSON number, +// and `uint` as a JSON string. +// +// In this file, we're overriding LegacyAminoPubKey's default Amino marshaling +// by using this struct. +// +// ref: https://github.com/cosmos/cosmos-sdk/issues/8776 +type tmMultisig struct { + K uint `json:"threshold"` + PubKeys []cryptotypes.PubKey `json:"pubkeys"` +} + +// protoToTm converts a LegacyAminoPubKey into a tmMultisig. +func protoToTm(protoPk *LegacyAminoPubKey) (tmMultisig, error) { + var ok bool + pks := make([]cryptotypes.PubKey, len(protoPk.PubKeys)) + for i, pk := range protoPk.PubKeys { + pks[i], ok = pk.GetCachedValue().(cryptotypes.PubKey) + if !ok { + return tmMultisig{}, sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", (cryptotypes.PubKey)(nil), pk.GetCachedValue()) + } + } + + return tmMultisig{ + K: uint(protoPk.Threshold), + PubKeys: pks, + }, nil +} + +// tmToProto converts a tmMultisig into a LegacyAminoPubKey. +func tmToProto(tmPk tmMultisig) (*LegacyAminoPubKey, error) { + var err error + pks := make([]*types.Any, len(tmPk.PubKeys)) + for i, pk := range tmPk.PubKeys { + pks[i], err = types.NewAnyWithValue(pk) + if err != nil { + return nil, err + } + } + + return &LegacyAminoPubKey{ + Threshold: uint32(tmPk.K), + PubKeys: pks, + }, nil +} + +// MarshalAmino overrides amino binary unmarshaling. +func (m LegacyAminoPubKey) MarshalAmino() (tmMultisig, error) { + return protoToTm(&m) +} + +// UnmarshalAmino overrides amino binary unmarshaling. +func (m *LegacyAminoPubKey) UnmarshalAmino(tmPk tmMultisig) error { + protoPk, err := tmToProto(tmPk) + if err != nil { + return err + } + + *m = *protoPk + + return nil +} + +// MarshalAminoJSON overrides amino JSON unmarshaling. +func (m LegacyAminoPubKey) MarshalAminoJSON() (tmMultisig, error) { + return protoToTm(&m) +} + +// UnmarshalAminoJSON overrides amino JSON unmarshaling. +func (m *LegacyAminoPubKey) UnmarshalAminoJSON(tmPk tmMultisig) error { + protoPk, err := tmToProto(tmPk) + if err != nil { + return err + } + + *m = *protoPk + + return nil +} diff --git a/crypto/keys/multisig/multisig_test.go b/crypto/keys/multisig/multisig_test.go index 69b4f10d269..197a433a72e 100644 --- a/crypto/keys/multisig/multisig_test.go +++ b/crypto/keys/multisig/multisig_test.go @@ -6,7 +6,9 @@ import ( "github.com/stretchr/testify/require" "github.com/cosmos/cosmos-sdk/codec" + "github.com/cosmos/cosmos-sdk/codec/legacy" "github.com/cosmos/cosmos-sdk/codec/types" + cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" 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" @@ -272,12 +274,12 @@ func TestPubKeyMultisigThresholdAminoToIface(t *testing.T) { pubkeys, _ := generatePubKeysAndSignatures(5, msg) multisigKey := kmultisig.NewLegacyAminoPubKey(2, pubkeys) - ab, err := kmultisig.AminoCdc.MarshalBinaryLengthPrefixed(multisigKey) + ab, err := legacy.Cdc.MarshalBinaryLengthPrefixed(multisigKey) require.NoError(t, err) // like other cryptotypes.Pubkey implementations (e.g. ed25519.PubKey), // LegacyAminoPubKey should be deserializable into a cryptotypes.LegacyAminoPubKey: var pubKey kmultisig.LegacyAminoPubKey - err = kmultisig.AminoCdc.UnmarshalBinaryLengthPrefixed(ab, &pubKey) + err = legacy.Cdc.UnmarshalBinaryLengthPrefixed(ab, &pubKey) require.NoError(t, err) require.Equal(t, multisigKey.Equals(&pubKey), true) @@ -329,3 +331,75 @@ func reorderPubKey(pk *kmultisig.LegacyAminoPubKey) (other *kmultisig.LegacyAmin other = &kmultisig.LegacyAminoPubKey{Threshold: 2, PubKeys: pubkeysCpy} return } + +func TestAminoBinary(t *testing.T) { + pubKey1 := secp256k1.GenPrivKey().PubKey() + pubKey2 := secp256k1.GenPrivKey().PubKey() + multisigKey := kmultisig.NewLegacyAminoPubKey(2, []cryptotypes.PubKey{pubKey1, pubKey2}) + + // Do a round-trip key->bytes->key. + bz, err := legacy.Cdc.MarshalBinaryBare(multisigKey) + require.NoError(t, err) + var newMultisigKey cryptotypes.PubKey + err = legacy.Cdc.UnmarshalBinaryBare(bz, &newMultisigKey) + require.NoError(t, err) + require.Equal(t, multisigKey.Threshold, newMultisigKey.(*kmultisig.LegacyAminoPubKey).Threshold) +} + +func TestAminoMarshalJSON(t *testing.T) { + pubKey1 := secp256k1.GenPrivKey().PubKey() + pubKey2 := secp256k1.GenPrivKey().PubKey() + multisigKey := kmultisig.NewLegacyAminoPubKey(2, []cryptotypes.PubKey{pubKey1, pubKey2}) + + bz, err := legacy.Cdc.MarshalJSON(multisigKey) + require.NoError(t, err) + + // Note the quotes around `"2"`. They are present because we are overriding + // the Amino JSON marshaling of LegacyAminoPubKey (using tmMultisig). + // Without the override, there would not be any quotes. + require.Contains(t, string(bz), "\"threshold\":\"2\"") +} + +func TestAminoUnmarshalJSON(t *testing.T) { + // This is a real multisig from the Akash chain. It has been exported from + // v0.39, hence the `threshold` field as a string. + // We are testing that when unmarshaling this JSON into a LegacyAminoPubKey + // with amino, there's no error. + // ref: https://github.com/cosmos/cosmos-sdk/issues/8776 + pkJSON := `{ + "type": "tendermint/PubKeyMultisigThreshold", + "value": { + "pubkeys": [ + { + "type": "tendermint/PubKeySecp256k1", + "value": "AzYxq2VNeD10TyABwOgV36OVWDIMn8AtI4OFA0uQX2MK" + }, + { + "type": "tendermint/PubKeySecp256k1", + "value": "A39cdsrm00bTeQ3RVZVqjkH8MvIViO9o99c8iLiNO35h" + }, + { + "type": "tendermint/PubKeySecp256k1", + "value": "A/uLLCZph8MkFg2tCxqSMGwFfPHdt1kkObmmrqy9aiYD" + }, + { + "type": "tendermint/PubKeySecp256k1", + "value": "A4mOMhM5gPDtBAkAophjRs6uDGZm4tD4Dbok3ai4qJi8" + }, + { + "type": "tendermint/PubKeySecp256k1", + "value": "A90icFucrjNNz2SAdJWMApfSQcARIqt+M2x++t6w5fFs" + } + ], + "threshold": "3" + } +}` + + cdc := codec.NewLegacyAmino() + cryptocodec.RegisterCrypto(cdc) + + var pk cryptotypes.PubKey + err := cdc.UnmarshalJSON([]byte(pkJSON), &pk) + require.NoError(t, err) + require.Equal(t, uint32(3), pk.(*kmultisig.LegacyAminoPubKey).Threshold) +} From 1581314420e33e2d12b85f621e01b4bed85a5bf2 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 10 Mar 2021 15:01:43 +0100 Subject: [PATCH 03/13] Fix registercrypto --- x/auth/legacy/v034/types.go | 32 ++------------------------------ x/auth/legacy/v036/types.go | 3 ++- x/auth/legacy/v038/types.go | 3 ++- x/auth/legacy/v039/types.go | 3 ++- 4 files changed, 8 insertions(+), 33 deletions(-) diff --git a/x/auth/legacy/v034/types.go b/x/auth/legacy/v034/types.go index 70d040ff4db..6828c886ba0 100644 --- a/x/auth/legacy/v034/types.go +++ b/x/auth/legacy/v034/types.go @@ -3,12 +3,8 @@ package v034 import ( "github.com/cosmos/cosmos-sdk/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" + cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/tendermint/tendermint/crypto" ) const ( @@ -30,30 +26,6 @@ type ( } ) -// PubKeyMultisigThreshold implements a K of N threshold multisig. -// This struct is copy-pasted from: -// https://github.com/tendermint/tendermint/blob/v0.33.9/crypto/multisig/threshold_pubkey.go -type PubKeyMultisigThreshold struct { - K uint `json:"threshold"` - PubKeys []crypto.PubKey `json:"pubkeys"` -} - -func RegisterCrypto(cdc *codec.LegacyAmino) { - cdc.RegisterInterface((*cryptotypes.PubKey)(nil), nil) - cdc.RegisterConcrete(&ed25519.PubKey{}, - ed25519.PubKeyName, nil) - cdc.RegisterConcrete(&secp256k1.PubKey{}, - secp256k1.PubKeyName, nil) - cdc.RegisterConcrete(&PubKeyMultisigThreshold{}, - kmultisig.PubKeyAminoRoute, nil) - - cdc.RegisterInterface((*cryptotypes.PrivKey)(nil), nil) - cdc.RegisterConcrete(&ed25519.PrivKey{}, //nolint:staticcheck - ed25519.PrivKeyName, nil) - cdc.RegisterConcrete(&secp256k1.PrivKey{}, - secp256k1.PrivKeyName, nil) -} - func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { - RegisterCrypto(cdc) + cryptocodec.RegisterCrypto(cdc) } diff --git a/x/auth/legacy/v036/types.go b/x/auth/legacy/v036/types.go index dcef8779cd7..d87e82f6015 100644 --- a/x/auth/legacy/v036/types.go +++ b/x/auth/legacy/v036/types.go @@ -3,6 +3,7 @@ package v036 import ( "github.com/cosmos/cosmos-sdk/codec" + cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" v034auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v034" ) @@ -21,5 +22,5 @@ func NewGenesisState(params v034auth.Params) GenesisState { } func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { - v034auth.RegisterCrypto(cdc) + cryptocodec.RegisterCrypto(cdc) } diff --git a/x/auth/legacy/v038/types.go b/x/auth/legacy/v038/types.go index aec415bc7ce..b7dedf003e1 100644 --- a/x/auth/legacy/v038/types.go +++ b/x/auth/legacy/v038/types.go @@ -13,6 +13,7 @@ import ( tmcrypto "github.com/tendermint/tendermint/crypto" "github.com/cosmos/cosmos-sdk/codec" + cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" v034auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v034" @@ -517,7 +518,7 @@ func ValidateGenAccounts(genAccounts GenesisAccounts) error { } func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { - v034auth.RegisterCrypto(cdc) + cryptocodec.RegisterCrypto(cdc) cdc.RegisterInterface((*GenesisAccount)(nil), nil) cdc.RegisterInterface((*Account)(nil), nil) cdc.RegisterConcrete(&BaseAccount{}, "cosmos-sdk/Account", nil) diff --git a/x/auth/legacy/v039/types.go b/x/auth/legacy/v039/types.go index fbab9cf39fe..3de576f27ff 100644 --- a/x/auth/legacy/v039/types.go +++ b/x/auth/legacy/v039/types.go @@ -12,6 +12,7 @@ import ( "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/codec/legacy" + cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" sdk "github.com/cosmos/cosmos-sdk/types" v034auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v034" @@ -415,7 +416,7 @@ func (ma *ModuleAccount) UnmarshalJSON(bz []byte) error { } func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { - v034auth.RegisterCrypto(cdc) + cryptocodec.RegisterCrypto(cdc) cdc.RegisterInterface((*v038auth.GenesisAccount)(nil), nil) cdc.RegisterInterface((*v038auth.Account)(nil), nil) cdc.RegisterConcrete(&BaseAccount{}, "cosmos-sdk/Account", nil) From 4b2612fdd132bfa62fe7664dbd8f0f3449eeb9fe Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 10 Mar 2021 15:03:59 +0100 Subject: [PATCH 04/13] Revert old PR --- x/auth/legacy/v040/migrate.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/x/auth/legacy/v040/migrate.go b/x/auth/legacy/v040/migrate.go index f819a1a33d8..4ba3e0cb7c8 100644 --- a/x/auth/legacy/v040/migrate.go +++ b/x/auth/legacy/v040/migrate.go @@ -2,7 +2,6 @@ package v040 import ( codectypes "github.com/cosmos/cosmos-sdk/codec/types" - multisigtypes "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" v039auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v039" v040auth "github.com/cosmos/cosmos-sdk/x/auth/types" @@ -13,15 +12,7 @@ import ( func convertBaseAccount(old *v039auth.BaseAccount) *v040auth.BaseAccount { var any *codectypes.Any - _, ok := old.PubKey.(*multisigtypes.LegacyAminoPubKey) - - // If pubkey is multisig type, then leave it as nil for now - // Ref: https://github.com/cosmos/cosmos-sdk/issues/8776#issuecomment-790552126 - // Else if the old genesis had a pubkey, we pack it inside an Any. - // Or else, we just leave it nil. - if ok { - // TODO migrate multisig public_keys - } else if old.PubKey != nil { + if old.PubKey != nil { var err error any, err = codectypes.NewAnyWithValue(old.PubKey) if err != nil { From 331e48ef04a28d7e7159015d362a0ba2a9fd2215 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 10 Mar 2021 15:11:26 +0100 Subject: [PATCH 05/13] revert some genutil stuff --- x/auth/legacy/v034/types.go | 6 ------ x/auth/legacy/v036/types.go | 6 ------ x/genutil/legacy/v036/migrate.go | 5 +++-- x/genutil/legacy/v038/migrate.go | 3 ++- 4 files changed, 5 insertions(+), 15 deletions(-) diff --git a/x/auth/legacy/v034/types.go b/x/auth/legacy/v034/types.go index 6828c886ba0..83fa2d36eac 100644 --- a/x/auth/legacy/v034/types.go +++ b/x/auth/legacy/v034/types.go @@ -2,8 +2,6 @@ package v034 import ( - "github.com/cosmos/cosmos-sdk/codec" - cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -25,7 +23,3 @@ type ( Params Params `json:"params"` } ) - -func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { - cryptocodec.RegisterCrypto(cdc) -} diff --git a/x/auth/legacy/v036/types.go b/x/auth/legacy/v036/types.go index d87e82f6015..bbc238838d1 100644 --- a/x/auth/legacy/v036/types.go +++ b/x/auth/legacy/v036/types.go @@ -2,8 +2,6 @@ package v036 import ( - "github.com/cosmos/cosmos-sdk/codec" - cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" v034auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v034" ) @@ -20,7 +18,3 @@ type ( func NewGenesisState(params v034auth.Params) GenesisState { return GenesisState{params} } - -func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) { - cryptocodec.RegisterCrypto(cdc) -} diff --git a/x/genutil/legacy/v036/migrate.go b/x/genutil/legacy/v036/migrate.go index 204b1b6a39a..204eebc233e 100644 --- a/x/genutil/legacy/v036/migrate.go +++ b/x/genutil/legacy/v036/migrate.go @@ -3,6 +3,7 @@ package v036 import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" + cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" v034auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v034" v036auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v036" v036bank "github.com/cosmos/cosmos-sdk/x/bank/legacy/v036" @@ -21,11 +22,11 @@ import ( // Migrate migrates exported state from v0.34 to a v0.36 genesis state. func Migrate(appState types.AppMap, _ client.Context) types.AppMap { v034Codec := codec.NewLegacyAmino() - v034auth.RegisterLegacyAminoCodec(v034Codec) + cryptocodec.RegisterCrypto(v034Codec) v034gov.RegisterLegacyAminoCodec(v034Codec) v036Codec := codec.NewLegacyAmino() - v036auth.RegisterLegacyAminoCodec(v036Codec) + cryptocodec.RegisterCrypto(v036Codec) v036gov.RegisterLegacyAminoCodec(v036Codec) v036distr.RegisterLegacyAminoCodec(v036Codec) v036params.RegisterLegacyAminoCodec(v036Codec) diff --git a/x/genutil/legacy/v038/migrate.go b/x/genutil/legacy/v038/migrate.go index 74240787ab9..017d0e68fa5 100644 --- a/x/genutil/legacy/v038/migrate.go +++ b/x/genutil/legacy/v038/migrate.go @@ -3,6 +3,7 @@ package v038 import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" + cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" v036auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v036" v038auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v038" v036distr "github.com/cosmos/cosmos-sdk/x/distribution/legacy/v036" @@ -19,7 +20,7 @@ import ( // Migrate migrates exported state from v0.36/v0.37 to a v0.38 genesis state. func Migrate(appState types.AppMap, _ client.Context) types.AppMap { v036Codec := codec.NewLegacyAmino() - v036auth.RegisterLegacyAminoCodec(v036Codec) + cryptocodec.RegisterCrypto(v036Codec) v036gov.RegisterLegacyAminoCodec(v036Codec) v036distr.RegisterLegacyAminoCodec(v036Codec) v036params.RegisterLegacyAminoCodec(v036Codec) From b75009f52ae1b1aa3417757c70d367dc92e78e18 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 10 Mar 2021 15:15:01 +0100 Subject: [PATCH 06/13] Add comment --- crypto/keys/multisig/codec.go | 3 +++ x/auth/legacy/v036/types.go | 4 +--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/crypto/keys/multisig/codec.go b/crypto/keys/multisig/codec.go index b92576e4f3f..5e721e105d4 100644 --- a/crypto/keys/multisig/codec.go +++ b/crypto/keys/multisig/codec.go @@ -15,6 +15,9 @@ const ( PubKeyAminoRoute = "tendermint/PubKeyMultisigThreshold" ) +// AminoCdc is the legacy Amino codec. +// DEPRECATED: Amino is being deprecated in the SDK. But even if you need to +// use Amino for some reason, please use `codec/legacy.Cdc` instead. var AminoCdc = codec.NewLegacyAmino() func init() { diff --git a/x/auth/legacy/v036/types.go b/x/auth/legacy/v036/types.go index bbc238838d1..908165f2650 100644 --- a/x/auth/legacy/v036/types.go +++ b/x/auth/legacy/v036/types.go @@ -1,9 +1,7 @@ // DONTCOVER package v036 -import ( - v034auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v034" -) +import v034auth "github.com/cosmos/cosmos-sdk/x/auth/legacy/v034" const ( ModuleName = "auth" From ec9108322f7fdfd09c691090b8e258878c3a07e3 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 10 Mar 2021 15:17:38 +0100 Subject: [PATCH 07/13] Add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39a231c74a5..521e61586a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -87,6 +87,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/bank) [\#8434](https://github.com/cosmos/cosmos-sdk/pull/8434) Fix legacy REST API `GET /bank/total` and `GET /bank/total/{denom}` in swagger * (x/slashing) [\#8427](https://github.com/cosmos/cosmos-sdk/pull/8427) Fix query signing infos command * (server) [\#8399](https://github.com/cosmos/cosmos-sdk/pull/8399) fix gRPC-web flag default value +* (crypto) [\#8841](https://github.com/cosmos/cosmos-sdk/pull/8841) Fix legacy multisig amino marshaling, allowing migrations to work between v0.39 and v0.40+. ## [v0.42.0](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.42.0) - 2021-03-08 From 5114ef956a628648f313f7e415e06aae9437d97c Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 10 Mar 2021 15:31:07 +0100 Subject: [PATCH 08/13] Remove binary marshalling --- crypto/keys/multisig/amino.go | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/crypto/keys/multisig/amino.go b/crypto/keys/multisig/amino.go index d663df108ca..0c85b63b61b 100644 --- a/crypto/keys/multisig/amino.go +++ b/crypto/keys/multisig/amino.go @@ -7,7 +7,7 @@ import ( ) // tmMultisig implements a K of N threshold multisig. It is used for -// Amino marshaling of LegacyAminoPubKey (see below for details). +// Amino JSON marshaling of LegacyAminoPubKey (see below for details). // // This struct is copy-pasted from: // https://github.com/tendermint/tendermint/blob/v0.33.9/crypto/multisig/threshold_pubkey.go @@ -18,8 +18,10 @@ import ( // amino marshaling to be breaking: amino marshals `uint32` as a JSON number, // and `uint` as a JSON string. // -// In this file, we're overriding LegacyAminoPubKey's default Amino marshaling -// by using this struct. +// In this file, we're overriding LegacyAminoPubKey's default JSON Amino +// marshaling by using this struct. Please note that we are NOT overriding the +// Amino binary marshaling, as that _might_ introduce breaking changes in the +// keyring, where multisigs are amino-binary-encoded. // // ref: https://github.com/cosmos/cosmos-sdk/issues/8776 type tmMultisig struct { @@ -61,23 +63,6 @@ func tmToProto(tmPk tmMultisig) (*LegacyAminoPubKey, error) { }, nil } -// MarshalAmino overrides amino binary unmarshaling. -func (m LegacyAminoPubKey) MarshalAmino() (tmMultisig, error) { - return protoToTm(&m) -} - -// UnmarshalAmino overrides amino binary unmarshaling. -func (m *LegacyAminoPubKey) UnmarshalAmino(tmPk tmMultisig) error { - protoPk, err := tmToProto(tmPk) - if err != nil { - return err - } - - *m = *protoPk - - return nil -} - // MarshalAminoJSON overrides amino JSON unmarshaling. func (m LegacyAminoPubKey) MarshalAminoJSON() (tmMultisig, error) { return protoToTm(&m) From a52d2cd7c58505bed1cbdeb8a4b7fda6116e0fee Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 10 Mar 2021 15:34:25 +0100 Subject: [PATCH 09/13] Fix lint --- crypto/keys/multisig/amino.go | 2 +- crypto/keys/multisig/codec.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crypto/keys/multisig/amino.go b/crypto/keys/multisig/amino.go index 0c85b63b61b..e979eb784da 100644 --- a/crypto/keys/multisig/amino.go +++ b/crypto/keys/multisig/amino.go @@ -64,7 +64,7 @@ func tmToProto(tmPk tmMultisig) (*LegacyAminoPubKey, error) { } // MarshalAminoJSON overrides amino JSON unmarshaling. -func (m LegacyAminoPubKey) MarshalAminoJSON() (tmMultisig, error) { +func (m LegacyAminoPubKey) MarshalAminoJSON() (tmMultisig, error) { //nolint:golint return protoToTm(&m) } diff --git a/crypto/keys/multisig/codec.go b/crypto/keys/multisig/codec.go index 5e721e105d4..f46d019aa2b 100644 --- a/crypto/keys/multisig/codec.go +++ b/crypto/keys/multisig/codec.go @@ -16,7 +16,7 @@ const ( ) // AminoCdc is the legacy Amino codec. -// DEPRECATED: Amino is being deprecated in the SDK. But even if you need to +// Deprecated: Amino is being deprecated in the SDK. But even if you need to // use Amino for some reason, please use `codec/legacy.Cdc` instead. var AminoCdc = codec.NewLegacyAmino() From 99d1cd4aa1007764e77dadd10ad8e5b427172e8c Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 10 Mar 2021 15:39:49 +0100 Subject: [PATCH 10/13] Fix lint again --- crypto/keys/multisig/codec.go | 1 + 1 file changed, 1 insertion(+) diff --git a/crypto/keys/multisig/codec.go b/crypto/keys/multisig/codec.go index f46d019aa2b..f522d1a7525 100644 --- a/crypto/keys/multisig/codec.go +++ b/crypto/keys/multisig/codec.go @@ -16,6 +16,7 @@ const ( ) // AminoCdc is the legacy Amino codec. +// // Deprecated: Amino is being deprecated in the SDK. But even if you need to // use Amino for some reason, please use `codec/legacy.Cdc` instead. var AminoCdc = codec.NewLegacyAmino() From b0ade08aeb314946b4d08480350f5f8b2e6afe25 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Wed, 10 Mar 2021 15:47:17 +0100 Subject: [PATCH 11/13] Fix lint, 3rd time's a charm --- crypto/keys/multisig/codec.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/crypto/keys/multisig/codec.go b/crypto/keys/multisig/codec.go index f522d1a7525..5bee9d8ed40 100644 --- a/crypto/keys/multisig/codec.go +++ b/crypto/keys/multisig/codec.go @@ -15,8 +15,6 @@ const ( PubKeyAminoRoute = "tendermint/PubKeyMultisigThreshold" ) -// AminoCdc is the legacy Amino codec. -// // Deprecated: Amino is being deprecated in the SDK. But even if you need to // use Amino for some reason, please use `codec/legacy.Cdc` instead. var AminoCdc = codec.NewLegacyAmino() From 8f938f00ebc5f5cde2e22da306be0bcc55116a43 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Thu, 11 Mar 2021 15:26:17 +0000 Subject: [PATCH 12/13] ignore wrong linter warning --- crypto/keys/multisig/codec.go | 1 + 1 file changed, 1 insertion(+) diff --git a/crypto/keys/multisig/codec.go b/crypto/keys/multisig/codec.go index 5bee9d8ed40..d501e1b427c 100644 --- a/crypto/keys/multisig/codec.go +++ b/crypto/keys/multisig/codec.go @@ -15,6 +15,7 @@ const ( PubKeyAminoRoute = "tendermint/PubKeyMultisigThreshold" ) +//nolint // Deprecated: Amino is being deprecated in the SDK. But even if you need to // use Amino for some reason, please use `codec/legacy.Cdc` instead. var AminoCdc = codec.NewLegacyAmino() From 6a785f03aa59f1b5cc1ab458b5dd58599ca44d08 Mon Sep 17 00:00:00 2001 From: Amaury M <1293565+amaurym@users.noreply.github.com> Date: Mon, 15 Mar 2021 12:48:25 +0100 Subject: [PATCH 13/13] Fix UnmarshalAmioJSON --- crypto/keys/multisig/amino.go | 9 ++++++++- x/auth/legacy/legacytx/stdtx.go | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/crypto/keys/multisig/amino.go b/crypto/keys/multisig/amino.go index e979eb784da..4849a23173d 100644 --- a/crypto/keys/multisig/amino.go +++ b/crypto/keys/multisig/amino.go @@ -75,7 +75,14 @@ func (m *LegacyAminoPubKey) UnmarshalAminoJSON(tmPk tmMultisig) error { return err } - *m = *protoPk + // Instead of just doing `*m = *protoPk`, we prefer to modify in-place the + // existing Anys inside `m` (instead of allocating new Anys), as so not to + // break the `.compat` fields in the existing Anys. + for i := range m.PubKeys { + m.PubKeys[i].TypeUrl = protoPk.PubKeys[i].TypeUrl + m.PubKeys[i].Value = protoPk.PubKeys[i].Value + } + m.Threshold = protoPk.Threshold return nil } diff --git a/x/auth/legacy/legacytx/stdtx.go b/x/auth/legacy/legacytx/stdtx.go index c1a982c85ef..c2b60736e4c 100644 --- a/x/auth/legacy/legacytx/stdtx.go +++ b/x/auth/legacy/legacytx/stdtx.go @@ -287,7 +287,7 @@ func (tx StdTx) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error { // Signatures contain PubKeys, which need to be unpacked. for _, s := range tx.Signatures { - err := codectypes.UnpackInterfaces(s, unpacker) + err := s.UnpackInterfaces(unpacker) if err != nil { return err }