From 2a1409b68fbbfb4d42852dbc1779df51431e932c Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Tue, 9 Nov 2021 13:48:45 +0100 Subject: [PATCH] fix: unmarshalling issue with multisig keys in master (backport #10061) (#10113) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: unmarshalling issue with multisig keys in master (#10061) (cherry picked from commit 3d3bc7c43218c37c6d0033d578cc7b24968a9be2) # Conflicts: # CHANGELOG.md # crypto/keys/multisig/multisig_test.go * fix conflicts * fix conflicts * Update crypto/keys/multisig/multisig_test.go * Removed unused imports * fix changelog * Update CHANGELOG.md * Fix TestLegacyMultisig test (#10275) Co-authored-by: Henrik Aasted Sørensen Co-authored-by: Aleksandr Bezobchuk Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> Co-authored-by: Robert Zaremba Co-authored-by: Mario Karagiorgas --- CHANGELOG.md | 3 ++- crypto/keys/multisig/amino.go | 23 ++++++++++++++-- crypto/keys/multisig/multisig_test.go | 38 ++++++++++++++++++++++++++- 3 files changed, 60 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 554c72a36fcc..c9f009c0a82e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes * [\#9969](https://github.com/cosmos/cosmos-sdk/pull/9969) fix: use keyring in config for add-genesis-account cmd. +* [\#10061](https://github.com/cosmos/cosmos-sdk/pull/10061) Ensure that `LegacyAminoPubKey` struct correctly unmarshals from JSON ### Client Breaking Changes @@ -64,7 +65,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### API Breaking Changes -* [\#10077](https://github.com/cosmos/cosmos-sdk/pull/10077) Remove telemetry on `GasKV` and `CacheKV` store Get/Set operations, significantly improving their performance. ++ [\#10077](https://github.com/cosmos/cosmos-sdk/pull/10077) Remove telemetry on `GasKV` and `CacheKV` store Get/Set operations, significantly improving their performance. ## [v0.42.9](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.42.9) - 2021-08-04 diff --git a/crypto/keys/multisig/amino.go b/crypto/keys/multisig/amino.go index 4849a23173d2..641fe1f691a7 100644 --- a/crypto/keys/multisig/amino.go +++ b/crypto/keys/multisig/amino.go @@ -78,9 +78,28 @@ func (m *LegacyAminoPubKey) UnmarshalAminoJSON(tmPk tmMultisig) error { // 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. + if m.PubKeys == nil { + m.PubKeys = make([]*types.Any, len(tmPk.PubKeys)) + } for i := range m.PubKeys { - m.PubKeys[i].TypeUrl = protoPk.PubKeys[i].TypeUrl - m.PubKeys[i].Value = protoPk.PubKeys[i].Value + if m.PubKeys[i] == nil { + // create the compat jsonBz value + bz, err := AminoCdc.MarshalJSON(tmPk.PubKeys[i]) + if err != nil { + return err + } + + m.PubKeys[i] = protoPk.PubKeys[i] + // UnmarshalJSON(): + // just sets the compat.jsonBz value. + // always succeeds: err == nil + if err := m.PubKeys[i].UnmarshalJSON(bz); err != nil { + return err + } + } else { + m.PubKeys[i].TypeUrl = protoPk.PubKeys[i].TypeUrl + m.PubKeys[i].Value = protoPk.PubKeys[i].Value + } } m.Threshold = protoPk.Threshold diff --git a/crypto/keys/multisig/multisig_test.go b/crypto/keys/multisig/multisig_test.go index 1284c6c88476..0285ae0ef9ae 100644 --- a/crypto/keys/multisig/multisig_test.go +++ b/crypto/keys/multisig/multisig_test.go @@ -17,6 +17,15 @@ import ( "github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx" ) +func generatePubKeys(n int) []cryptotypes.PubKey { + pks := make([]cryptotypes.PubKey, n) + for i := 0; i < n; i++ { + pks[i] = secp256k1.GenPrivKey().PubKey() + } + + return pks +} + func TestAddress(t *testing.T) { msg := []byte{1, 2, 3, 4} pubKeys, _ := generatePubKeysAndSignatures(5, msg) @@ -379,5 +388,32 @@ func TestAminoUnmarshalJSON(t *testing.T) { var pk cryptotypes.PubKey err := cdc.UnmarshalJSON([]byte(pkJSON), &pk) require.NoError(t, err) - require.Equal(t, uint32(3), pk.(*kmultisig.LegacyAminoPubKey).Threshold) + lpk := pk.(*kmultisig.LegacyAminoPubKey) + require.Equal(t, uint32(3), lpk.Threshold) + require.Equal(t, 5, len(pk.(*kmultisig.LegacyAminoPubKey).PubKeys)) + + for _, key := range pk.(*kmultisig.LegacyAminoPubKey).PubKeys { + require.NotNil(t, key) + pk := secp256k1.PubKey{} + err := pk.Unmarshal(key.Value) + require.NoError(t, err) + } +} + +func TestProtoMarshalJSON(t *testing.T) { + require := require.New(t) + pubkeys := generatePubKeys(3) + msig := kmultisig.NewLegacyAminoPubKey(2, pubkeys) + + registry := types.NewInterfaceRegistry() + cryptocodec.RegisterInterfaces(registry) + cdc := codec.NewProtoCodec(registry) + + bz, err := cdc.MarshalInterfaceJSON(msig) + require.NoError(err) + + var pk2 cryptotypes.PubKey + err = cdc.UnmarshalInterfaceJSON(bz, &pk2) + require.NoError(err) + require.True(pk2.Equals(msig)) }