LaconicNetwork/cosmos-sdk
7
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

Fix multisig LegacyAminoPubKey Amino marshaling (#8841)

Browse Source 
* Use v034auth RegisterCrypto

* Add custom amino for LegacyAminoPubKey

* Fix registercrypto

* Revert old PR

* revert some genutil stuff

* Add comment

* Add changelog

* Remove binary marshalling

* Fix lint

* Fix lint again

* Fix lint, 3rd time's a charm

* ignore wrong linter warning

* Fix UnmarshalAmioJSON

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Alessio Treglia <alessio@tendermint.com>
Co-authored-by: Anil Kumar Kammari <anil@vitwit.com>
This commit is contained in:
Amaury 2021-03-15 13:42:10 +01:00 committed by GitHub
parent 5f71e93b1e
commit d4d27e1c0c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 172 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -86,6 +86,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

88
crypto/keys/multisig/amino.go Normal file
View File

@ -0,0 +1,88 @@
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 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
//
// 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 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 {
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
}
// MarshalAminoJSON overrides amino JSON unmarshaling.
func (m LegacyAminoPubKey) MarshalAminoJSON() (tmMultisig, error) { //nolint:golint
return protoToTm(&m)
}
// UnmarshalAminoJSON overrides amino JSON unmarshaling.
func (m *LegacyAminoPubKey) UnmarshalAminoJSON(tmPk tmMultisig) error {
protoPk, err := tmToProto(tmPk)
if err != nil {
return err
}
// 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
}

3
crypto/keys/multisig/codec.go
View File

@ -15,6 +15,9 @@ 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()
func init() {

78
crypto/keys/multisig/multisig_test.go
View File

@ -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)
}

2
x/auth/legacy/legacytx/stdtx.go
View File

@ -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
}

11
x/auth/legacy/v040/migrate.go
View File

@ -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 {

2
x/genutil/legacy/v038/migrate.go
View File

@ -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,6 +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()
cryptocodec.RegisterCrypto(v036Codec)
v036gov.RegisterLegacyAminoCodec(v036Codec)
v036distr.RegisterLegacyAminoCodec(v036Codec)
v036params.RegisterLegacyAminoCodec(v036Codec)