From 23578a9612520c5c55d96683b7eb022d5b0f809b Mon Sep 17 00:00:00 2001 From: Marie Date: Fri, 18 Sep 2020 11:40:39 +0200 Subject: [PATCH] Remove old PubKeyMultisigThreshold (#7284) * WIP on protobuf keys * Use Type() and Bytes() in sr25519 pub key Equals * Add tests * Add few more tests * Update other pub/priv key types Equals * Fix PrivKey's Sign method * Rename variables in tests * Fix infinite recursive calls * Use tm ed25519 keys * Add Sign and VerifySignature tests * Remove ed25519 and sr25519 references * proto linting * Add proto crypto file * Implement some of the new multisig proto type methods * Add tests for MultisigThresholdPubKey * Add tests for pubkey pb/amino conversion functions * Move crypto types.go and register new proto pubkeys * Add missing pointer ref * Address review comments * panic in MultisigThresholdPubKey VerifySignature * Use internal crypto.PubKey in multisig * Add tests for MultisigThresholdPubKey VerifyMultisignature * Only keep LegacyAminoMultisigThresholdPubKey and move to proto keys to v1 * Remove conversion functions and introduce internal PubKey type * Start removal of old PubKeyMultisigThreshold references * Remove old secp256k1 PubKey and PrivKey * Uncomment test case * Fix linting issues * More linting * Revert tests keys values * Add Amino overrides to proto keys * Add pubkey test * Fix tests * Use threshold isntead of K * Standardize Type * Revert standardize types commit * Add comment * Simplify proto names * Fixed merge issues * Uncomment tests * Remove old multisig * Add amino marshal for multisig * Fix lint * Correctly register amino * One test left! * Remove old struct * Fix test * Fix test * Unpack into tmcrypto * Remove old threshold pubkey tests * Fix register amino * Fix lint * Use sdk crypto PubKey in multisig UnpackInterfaces * Potential fix? * Use anil's suggestion Co-authored-by: Aaron Craelius Co-authored-by: Amaury Martiny Co-authored-by: Alexander Bezobchuk Co-authored-by: Alessio Treglia --- client/keys/add.go | 4 +- client/keys/show.go | 4 +- client/keys/show_test.go | 7 +- crypto/codec/amino.go | 13 +- crypto/keyring/info.go | 35 +- crypto/keyring/keyring_test.go | 19 +- crypto/keyring/output_test.go | 4 +- crypto/keys/multisig/codec.go | 35 ++ crypto/keys/multisig/multisig.go | 65 +++- crypto/keys/multisig/multisig_test.go | 202 +++++++++--- crypto/types/multisig/codec.go | 30 -- crypto/types/multisig/threshold_pubkey.go | 164 --------- .../types/multisig/threshold_pubkey_test.go | 311 ------------------ std/pubkey.go | 14 +- std/pubkey_test.go | 14 +- x/auth/ante/ante_test.go | 10 +- x/auth/ante/basic.go | 4 +- x/auth/ante/sigverify_test.go | 3 +- x/auth/client/cli/cli_test.go | 4 +- x/auth/client/cli/tx_multisign.go | 5 +- x/auth/signing/verify_test.go | 3 +- x/auth/testutil/suite.go | 9 +- x/auth/types/stdtx.go | 5 +- x/auth/types/stdtx_test.go | 4 +- 24 files changed, 337 insertions(+), 631 deletions(-) create mode 100644 crypto/keys/multisig/codec.go delete mode 100644 crypto/types/multisig/codec.go delete mode 100644 crypto/types/multisig/threshold_pubkey.go delete mode 100644 crypto/types/multisig/threshold_pubkey_test.go diff --git a/client/keys/add.go b/client/keys/add.go index 7b25ebe3fc..0be9b4970f 100644 --- a/client/keys/add.go +++ b/client/keys/add.go @@ -16,7 +16,7 @@ import ( "github.com/cosmos/cosmos-sdk/client/input" "github.com/cosmos/cosmos-sdk/crypto/hd" "github.com/cosmos/cosmos-sdk/crypto/keyring" - "github.com/cosmos/cosmos-sdk/crypto/types/multisig" + "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -174,7 +174,7 @@ func RunAddCmd(cmd *cobra.Command, args []string, kb keyring.Keyring, inBuf *buf }) } - pk := multisig.NewPubKeyMultisigThreshold(multisigThreshold, pks) + pk := multisig.NewLegacyAminoPubKey(multisigThreshold, pks) if _, err := kb.SaveMultisig(name, pk); err != nil { return err } diff --git a/client/keys/show.go b/client/keys/show.go index 3899e1ea3f..ffc30342bf 100644 --- a/client/keys/show.go +++ b/client/keys/show.go @@ -10,8 +10,8 @@ import ( "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/crypto/keyring" + "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/ledger" - "github.com/cosmos/cosmos-sdk/crypto/types/multisig" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -83,7 +83,7 @@ func runShowCmd(cmd *cobra.Command, args []string) (err error) { return err } - multikey := multisig.NewPubKeyMultisigThreshold(multisigThreshold, pks) + multikey := multisig.NewLegacyAminoPubKey(multisigThreshold, pks) info = keyring.NewMultiInfo(defaultMultiSigKeyName, multikey) } diff --git a/client/keys/show_test.go b/client/keys/show_test.go index a186442079..9760b92d70 100644 --- a/client/keys/show_test.go +++ b/client/keys/show_test.go @@ -11,15 +11,18 @@ import ( "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/crypto/hd" "github.com/cosmos/cosmos-sdk/crypto/keyring" + "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" - "github.com/cosmos/cosmos-sdk/crypto/types/multisig" "github.com/cosmos/cosmos-sdk/testutil" sdk "github.com/cosmos/cosmos-sdk/types" ) func Test_multiSigKey_Properties(t *testing.T) { tmpKey1 := secp256k1.GenPrivKeyFromSecret([]byte("mySecret")) - pk := multisig.NewPubKeyMultisigThreshold(1, []crypto.PubKey{tmpKey1.PubKey()}) + pk := multisig.NewLegacyAminoPubKey( + 1, + []crypto.PubKey{tmpKey1.PubKey()}, + ) tmp := keyring.NewMultiInfo("myMultisig", pk) require.Equal(t, "myMultisig", tmp.GetName()) diff --git a/crypto/codec/amino.go b/crypto/codec/amino.go index abc3313778..c3ef815d40 100644 --- a/crypto/codec/amino.go +++ b/crypto/codec/amino.go @@ -8,7 +8,7 @@ import ( "github.com/cosmos/cosmos-sdk/codec" kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" - "github.com/cosmos/cosmos-sdk/crypto/types/multisig" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" ) var amino *codec.LegacyAmino @@ -21,20 +21,19 @@ func init() { // RegisterCrypto registers all crypto dependency types with the provided Amino // codec. func RegisterCrypto(cdc *codec.LegacyAmino) { + // TODO We now register both Tendermint's PubKey and our own PubKey. In the + // long-term, we should move away from Tendermint's PubKey, and delete this + // first line. cdc.RegisterInterface((*crypto.PubKey)(nil), nil) + cdc.RegisterInterface((*cryptotypes.PubKey)(nil), nil) cdc.RegisterConcrete(ed25519.PubKey{}, ed25519.PubKeyName, nil) cdc.RegisterConcrete(sr25519.PubKey{}, sr25519.PubKeyName, nil) cdc.RegisterConcrete(&secp256k1.PubKey{}, secp256k1.PubKeyName, nil) - // TODO Follow-up in https://github.com/cosmos/cosmos-sdk/pull/7284 - // Remove `multisig.PubKeyMultisigThreshold{}`, and register instead - // kmultisig.LegacyAminoPubKey{} on `PubKeyAminoRoute`. - cdc.RegisterConcrete(multisig.PubKeyMultisigThreshold{}, - multisig.PubKeyAminoRoute, nil) cdc.RegisterConcrete(&kmultisig.LegacyAminoPubKey{}, - "cosmos-sdk/LegacyAminoPubKey", nil) + kmultisig.PubKeyAminoRoute, nil) cdc.RegisterInterface((*crypto.PrivKey)(nil), nil) cdc.RegisterConcrete(ed25519.PrivKey{}, diff --git a/crypto/keyring/info.go b/crypto/keyring/info.go index 74bb62d774..366e474d22 100644 --- a/crypto/keyring/info.go +++ b/crypto/keyring/info.go @@ -5,8 +5,9 @@ import ( "github.com/tendermint/tendermint/crypto" + codectypes "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/crypto/hd" - "github.com/cosmos/cosmos-sdk/crypto/types/multisig" + "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/types" ) @@ -191,10 +192,10 @@ type multiInfo struct { // NewMultiInfo creates a new multiInfo instance func NewMultiInfo(name string, pub crypto.PubKey) Info { - multiPK := pub.(multisig.PubKeyMultisigThreshold) + multiPK := pub.(*multisig.LegacyAminoPubKey) pubKeys := make([]multisigPubKeyInfo, len(multiPK.PubKeys)) - for i, pk := range multiPK.PubKeys { + for i, pk := range multiPK.GetPubKeys() { // TODO: Recursively check pk for total weight? pubKeys[i] = multisigPubKeyInfo{pk, 1} } @@ -202,7 +203,7 @@ func NewMultiInfo(name string, pub crypto.PubKey) Info { return &multiInfo{ Name: name, PubKey: pub, - Threshold: multiPK.K, + Threshold: uint(multiPK.Threshold), PubKeys: pubKeys, } } @@ -237,6 +238,13 @@ func (i multiInfo) GetPath() (*hd.BIP44Params, error) { return nil, fmt.Errorf("BIP44 Paths are not available for this type") } +// UnpackInterfaces implements UnpackInterfacesMessage.UnpackInterfaces +func (i multiInfo) UnpackInterfaces(unpacker codectypes.AnyUnpacker) error { + multiPK := i.PubKey.(*multisig.LegacyAminoPubKey) + + return codectypes.UnpackInterfaces(multiPK, unpacker) +} + // encoding info func marshalInfo(i Info) []byte { return CryptoCdc.MustMarshalBinaryLengthPrefixed(i) @@ -245,5 +253,24 @@ func marshalInfo(i Info) []byte { // decoding info func unmarshalInfo(bz []byte) (info Info, err error) { err = CryptoCdc.UnmarshalBinaryLengthPrefixed(bz, &info) + if err != nil { + return nil, err + } + + // After unmarshalling into &info, if we notice that the info is a + // multiInfo, then we unmarshal again, explicitly in a multiInfo this time. + // Since multiInfo implements UnpackInterfacesMessage, this will correctly + // unpack the underlying anys inside the multiInfo. + // + // This is a workaround, as go cannot check that an interface (Info) + // implements another interface (UnpackInterfacesMessage). + _, ok := info.(multiInfo) + if ok { + var multi multiInfo + err = CryptoCdc.UnmarshalBinaryLengthPrefixed(bz, &multi) + + return multi, err + } + return } diff --git a/crypto/keyring/keyring_test.go b/crypto/keyring/keyring_test.go index e586fe38a0..ac2ba0381c 100644 --- a/crypto/keyring/keyring_test.go +++ b/crypto/keyring/keyring_test.go @@ -13,8 +13,8 @@ import ( "github.com/cosmos/cosmos-sdk/crypto" "github.com/cosmos/cosmos-sdk/crypto/hd" + "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" - "github.com/cosmos/cosmos-sdk/crypto/types/multisig" "github.com/cosmos/cosmos-sdk/testutil" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -390,10 +390,11 @@ func TestInMemoryLanguage(t *testing.T) { func TestInMemoryCreateMultisig(t *testing.T) { kb, err := New("keybasename", "memory", "", nil) require.NoError(t, err) - multi := multisig.PubKeyMultisigThreshold{ - K: 1, - PubKeys: []tmcrypto.PubKey{secp256k1.GenPrivKey().PubKey()}, - } + multi := multisig.NewLegacyAminoPubKey( + 1, []tmcrypto.PubKey{ + secp256k1.GenPrivKey().PubKey(), + }, + ) _, err = kb.SaveMultisig("multi", multi) require.NoError(t, err) } @@ -980,7 +981,13 @@ func TestAltKeyring_SaveMultisig(t *testing.T) { require.NoError(t, err) key := "multi" - pub := multisig.NewPubKeyMultisigThreshold(2, []tmcrypto.PubKey{mnemonic1.GetPubKey(), mnemonic2.GetPubKey()}) + pub := multisig.NewLegacyAminoPubKey( + 2, + []tmcrypto.PubKey{ + &secp256k1.PubKey{Key: mnemonic1.GetPubKey().Bytes()}, + &secp256k1.PubKey{Key: mnemonic2.GetPubKey().Bytes()}, + }, + ) info, err := keyring.SaveMultisig(key, pub) require.Nil(t, err) diff --git a/crypto/keyring/output_test.go b/crypto/keyring/output_test.go index dd92a0e1d0..dc98ad62bc 100644 --- a/crypto/keyring/output_test.go +++ b/crypto/keyring/output_test.go @@ -6,8 +6,8 @@ import ( "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/crypto" + kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" - "github.com/cosmos/cosmos-sdk/crypto/types/multisig" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -16,7 +16,7 @@ func TestBech32KeysOutput(t *testing.T) { bechTmpKey := sdk.MustBech32ifyPubKey(sdk.Bech32PubKeyTypeAccPub, tmpKey) tmpAddr := sdk.AccAddress(tmpKey.Address().Bytes()) - multisigPks := multisig.NewPubKeyMultisigThreshold(1, []crypto.PubKey{tmpKey}) + multisigPks := kmultisig.NewLegacyAminoPubKey(1, []crypto.PubKey{tmpKey}) multiInfo := NewMultiInfo("multisig", multisigPks) accAddr := sdk.AccAddress(multiInfo.GetPubKey().Address().Bytes()) bechPubKey := sdk.MustBech32ifyPubKey(sdk.Bech32PubKeyTypeAccPub, multiInfo.GetPubKey()) diff --git a/crypto/keys/multisig/codec.go b/crypto/keys/multisig/codec.go new file mode 100644 index 0000000000..aedb6bc0ac --- /dev/null +++ b/crypto/keys/multisig/codec.go @@ -0,0 +1,35 @@ +package multisig + +import ( + "github.com/tendermint/tendermint/crypto" + "github.com/tendermint/tendermint/crypto/ed25519" + "github.com/tendermint/tendermint/crypto/sr25519" + + "github.com/cosmos/cosmos-sdk/codec" + "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" +) + +// TODO: Figure out API for others to either add their own pubkey types, or +// to make verify / marshal accept a AminoCdc. +const ( + PubKeyAminoRoute = "tendermint/PubKeyMultisigThreshold" +) + +var AminoCdc = codec.NewLegacyAmino() + +func init() { + // TODO We now register both Tendermint's PubKey and our own PubKey. In the + // long-term, we should move away from Tendermint's PubKey, and delete this + // first line. + AminoCdc.RegisterInterface((*crypto.PubKey)(nil), nil) + AminoCdc.RegisterInterface((*cryptotypes.PubKey)(nil), nil) + AminoCdc.RegisterConcrete(ed25519.PubKey{}, + ed25519.PubKeyName, nil) + AminoCdc.RegisterConcrete(sr25519.PubKey{}, + sr25519.PubKeyName, nil) + AminoCdc.RegisterConcrete(&secp256k1.PubKey{}, + secp256k1.PubKeyName, nil) + AminoCdc.RegisterConcrete(&LegacyAminoPubKey{}, + PubKeyAminoRoute, nil) +} diff --git a/crypto/keys/multisig/multisig.go b/crypto/keys/multisig/multisig.go index 628b7ec06b..b4b3eb870d 100644 --- a/crypto/keys/multisig/multisig.go +++ b/crypto/keys/multisig/multisig.go @@ -3,34 +3,46 @@ package multisig import ( fmt "fmt" - "github.com/cosmos/cosmos-sdk/codec" + tmcrypto "github.com/tendermint/tendermint/crypto" "github.com/cosmos/cosmos-sdk/codec/types" crypto "github.com/cosmos/cosmos-sdk/crypto/types" + multisigtypes "github.com/cosmos/cosmos-sdk/crypto/types/multisig" "github.com/cosmos/cosmos-sdk/types/tx/signing" - tmcrypto "github.com/tendermint/tendermint/crypto" - - "github.com/cosmos/cosmos-sdk/crypto/types/multisig" + proto "github.com/gogo/protobuf/proto" ) -// In the refactor in https://github.com/cosmos/cosmos-sdk/pull/7284, make sure -// to use AminoCdc here. -var cdc = codec.NewProtoCodec(types.NewInterfaceRegistry()) +var _ multisigtypes.PubKey = &LegacyAminoPubKey{} +var _ types.UnpackInterfacesMessage = &LegacyAminoPubKey{} -var _ multisig.PubKey = &LegacyAminoPubKey{} +// NewLegacyAminoPubKey returns a new LegacyAminoPubKey. +// Panics if len(pubKeys) < k or 0 >= k. +func NewLegacyAminoPubKey(k int, pubKeys []tmcrypto.PubKey) *LegacyAminoPubKey { + if k <= 0 { + panic("threshold k of n multisignature: k <= 0") + } + if len(pubKeys) < k { + panic("threshold k of n multisignature: len(pubKeys) < k") + } + anyPubKeys, err := packPubKeys(pubKeys) + if err != nil { + panic(err) + } + return &LegacyAminoPubKey{Threshold: uint32(k), PubKeys: anyPubKeys} +} // Address implements crypto.PubKey Address method -func (m *LegacyAminoPubKey) Address() crypto.Address { +func (m *LegacyAminoPubKey) Address() tmcrypto.Address { return tmcrypto.AddressHash(m.Bytes()) } // Bytes returns the proto encoded version of the LegacyAminoPubKey func (m *LegacyAminoPubKey) Bytes() []byte { - return cdc.MustMarshalBinaryBare(m) + return AminoCdc.MustMarshalBinaryBare(m) } -// VerifyMultisignature implements the multisig.PubKey VerifyMultisignature method -func (m *LegacyAminoPubKey) VerifyMultisignature(getSignBytes multisig.GetSignBytesFunc, sig *signing.MultiSignatureData) error { +// VerifyMultisignature implements the multisigtypes.PubKey VerifyMultisignature method +func (m *LegacyAminoPubKey) VerifyMultisignature(getSignBytes multisigtypes.GetSignBytesFunc, sig *signing.MultiSignatureData) error { bitarray := sig.BitArray sigs := sig.Signatures size := bitarray.Count() @@ -62,7 +74,7 @@ func (m *LegacyAminoPubKey) VerifyMultisignature(getSignBytes multisig.GetSignBy return err } case *signing.MultiSignatureData: - nestedMultisigPk, ok := pubKeys[i].(multisig.PubKey) + nestedMultisigPk, ok := pubKeys[i].(multisigtypes.PubKey) if !ok { return fmt.Errorf("unable to parse pubkey of index %d", i) } @@ -101,7 +113,7 @@ func (m *LegacyAminoPubKey) GetPubKeys() []tmcrypto.PubKey { // Equals returns true if m and other both have the same number of keys, and // all constituent keys are the same, and in the same order. func (m *LegacyAminoPubKey) Equals(key tmcrypto.PubKey) bool { - otherKey, ok := key.(multisig.PubKey) + otherKey, ok := key.(multisigtypes.PubKey) if !ok { return false } @@ -128,3 +140,28 @@ func (m *LegacyAminoPubKey) GetThreshold() uint { func (m *LegacyAminoPubKey) Type() string { return "PubKeyMultisigThreshold" } + +// UnpackInterfaces implements UnpackInterfacesMessage.UnpackInterfaces +func (m *LegacyAminoPubKey) UnpackInterfaces(unpacker types.AnyUnpacker) error { + for _, any := range m.PubKeys { + var pk crypto.PubKey + err := unpacker.UnpackAny(any, &pk) + if err != nil { + return err + } + } + return nil +} + +func packPubKeys(pubKeys []tmcrypto.PubKey) ([]*types.Any, error) { + anyPubKeys := make([]*types.Any, len(pubKeys)) + + for i := 0; i < len(pubKeys); i++ { + any, err := types.NewAnyWithValue(pubKeys[i].(proto.Message)) + if err != nil { + return nil, err + } + anyPubKeys[i] = any + } + return anyPubKeys, nil +} diff --git a/crypto/keys/multisig/multisig_test.go b/crypto/keys/multisig/multisig_test.go index 9467207763..c418d42ceb 100644 --- a/crypto/keys/multisig/multisig_test.go +++ b/crypto/keys/multisig/multisig_test.go @@ -1,4 +1,4 @@ -package multisig +package multisig_test import ( "testing" @@ -6,9 +6,11 @@ import ( "github.com/cosmos/cosmos-sdk/codec/types" crypto "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/crypto/types/multisig" - proto "github.com/gogo/protobuf/proto" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" tmcrypto "github.com/tendermint/tendermint/crypto" + "github.com/cosmos/cosmos-sdk/codec" + kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" "github.com/cosmos/cosmos-sdk/types/tx/signing" "github.com/stretchr/testify/require" @@ -17,9 +19,7 @@ import ( func TestAddress(t *testing.T) { msg := []byte{1, 2, 3, 4} pubKeys, _ := generatePubKeysAndSignatures(5, msg) - anyPubKeys, err := packPubKeys(pubKeys) - require.NoError(t, err) - multisigKey := &LegacyAminoPubKey{Threshold: 2, PubKeys: anyPubKeys} + multisigKey := kmultisig.NewLegacyAminoPubKey(2, pubKeys) require.Len(t, multisigKey.Address().Bytes(), 20) } @@ -28,13 +28,8 @@ func TestEquals(t *testing.T) { pubKey1 := secp256k1.GenPrivKey().PubKey() pubKey2 := secp256k1.GenPrivKey().PubKey() - anyPubKeys, err := packPubKeys([]tmcrypto.PubKey{pubKey1, pubKey2}) - require.NoError(t, err) - multisigKey := &LegacyAminoPubKey{Threshold: 1, PubKeys: anyPubKeys} - - otherPubKeys, err := packPubKeys([]tmcrypto.PubKey{pubKey1, multisigKey}) - require.NoError(t, err) - otherMultisigKey := LegacyAminoPubKey{Threshold: 1, PubKeys: otherPubKeys} + multisigKey := kmultisig.NewLegacyAminoPubKey(1, []tmcrypto.PubKey{pubKey1, pubKey2}) + otherMultisigKey := kmultisig.NewLegacyAminoPubKey(1, []tmcrypto.PubKey{pubKey1, multisigKey}) testCases := []struct { msg string @@ -43,22 +38,22 @@ func TestEquals(t *testing.T) { }{ { "equals with proto pub key", - &LegacyAminoPubKey{Threshold: 1, PubKeys: anyPubKeys}, + &kmultisig.LegacyAminoPubKey{Threshold: 1, PubKeys: multisigKey.PubKeys}, true, }, { "different threshold", - &LegacyAminoPubKey{Threshold: 2, PubKeys: anyPubKeys}, + &kmultisig.LegacyAminoPubKey{Threshold: 2, PubKeys: multisigKey.PubKeys}, false, }, { "different pub keys length", - &LegacyAminoPubKey{Threshold: 1, PubKeys: []*types.Any{anyPubKeys[0]}}, + &kmultisig.LegacyAminoPubKey{Threshold: 1, PubKeys: []*types.Any{multisigKey.PubKeys[0]}}, false, }, { "different pub keys", - &otherMultisigKey, + otherMultisigKey, false, }, { @@ -66,6 +61,11 @@ func TestEquals(t *testing.T) { secp256k1.GenPrivKey().PubKey(), false, }, + { + "ensure that reordering pubkeys is treated as a different pubkey", + reorderPubKey(multisigKey), + false, + }, } for _, tc := range testCases { @@ -92,8 +92,7 @@ func TestVerifyMultisignature(t *testing.T) { { "nested multisignature", func() { - genPk, genSig, err := generateNestedMultiSignature(3, msg) - require.NoError(t, err) + genPk, genSig := generateNestedMultiSignature(3, msg) sig = genSig pk = genPk }, @@ -103,22 +102,18 @@ func TestVerifyMultisignature(t *testing.T) { "wrong size for sig bit array", func() { pubKeys, _ := generatePubKeysAndSignatures(3, msg) - anyPubKeys, err := packPubKeys(pubKeys) - require.NoError(t, err) - pk = &LegacyAminoPubKey{Threshold: 3, PubKeys: anyPubKeys} + pk = kmultisig.NewLegacyAminoPubKey(3, pubKeys) sig = multisig.NewMultisig(1) }, false, }, { - "single signature data", + "single signature data, expects the first k signatures to be valid", func() { k := 2 signingIndices := []int{0, 3, 1} pubKeys, sigs := generatePubKeysAndSignatures(5, msg) - anyPubKeys, err := packPubKeys(pubKeys) - require.NoError(t, err) - pk = &LegacyAminoPubKey{Threshold: uint32(k), PubKeys: anyPubKeys} + pk = kmultisig.NewLegacyAminoPubKey(k, pubKeys) sig = multisig.NewMultisig(len(pubKeys)) signBytesFn := func(mode signing.SignMode) ([]byte, error) { return msg, nil } @@ -137,6 +132,12 @@ func TestVerifyMultisignature(t *testing.T) { t, multisig.AddSignatureFromPubKey(sig, sigs[signingIndex], pubKeys[signingIndex], pubKeys), ) + require.Equal( + t, + i+1, + len(sig.Signatures), + "adding a signature for the same pubkey twice increased signature count by 2, index %d", i, + ) } require.Error( t, @@ -152,9 +153,64 @@ func TestVerifyMultisignature(t *testing.T) { pubKeys, ), ) + require.NoError( + t, + pk.VerifyMultisignature(signBytesFn, sig), + "multisig failed after k good signatures", + ) + + for i := k + 1; i < len(signingIndices); i++ { + signingIndex := signingIndices[i] + + require.NoError( + t, + multisig.AddSignatureFromPubKey( + sig, + sigs[signingIndex], + pubKeys[signingIndex], + pubKeys, + ), + ) + require.Equal( + t, + false, + pk.VerifyMultisignature(func(mode signing.SignMode) ([]byte, error) { + return msg, nil + }, sig), + "multisig didn't verify as expected after k sigs, i %d", i, + ) + require.NoError( + t, + multisig.AddSignatureFromPubKey( + sig, + sigs[signingIndex], + pubKeys[signingIndex], + pubKeys), + ) + require.Equal( + t, + i+1, + len(sig.Signatures), + "adding a signature for the same pubkey twice increased signature count by 2", + ) + } }, true, }, + { + "duplicate signatures", + func() { + pubKeys, sigs := generatePubKeysAndSignatures(5, msg) + pk = kmultisig.NewLegacyAminoPubKey(2, pubKeys) + sig = multisig.NewMultisig(5) + + require.Error(t, pk.VerifyMultisignature(signBytesFn, sig)) + multisig.AddSignatureFromPubKey(sig, sigs[0], pubKeys[0], pubKeys) + // Add second signature manually + sig.Signatures = append(sig.Signatures, sigs[0]) + }, + false, + }, } for _, tc := range testCases { @@ -170,6 +226,66 @@ func TestVerifyMultisignature(t *testing.T) { } } +func TestAddSignatureFromPubKeyNilCheck(t *testing.T) { + pkSet, sigs := generatePubKeysAndSignatures(5, []byte{1, 2, 3, 4}) + multisignature := multisig.NewMultisig(5) + + // verify no error is returned with all non-nil values + err := multisig.AddSignatureFromPubKey(multisignature, sigs[0], pkSet[0], pkSet) + require.NoError(t, err) + // verify error is returned when key value is nil + err = multisig.AddSignatureFromPubKey(multisignature, sigs[0], pkSet[0], nil) + require.Error(t, err) + // verify error is returned when pubkey value is nil + err = multisig.AddSignatureFromPubKey(multisignature, sigs[0], nil, pkSet) + require.Error(t, err) + // verify error is returned when signature value is nil + err = multisig.AddSignatureFromPubKey(multisignature, nil, pkSet[0], pkSet) + require.Error(t, err) + // verify error is returned when multisignature value is nil + err = multisig.AddSignatureFromPubKey(nil, sigs[0], pkSet[0], pkSet) + require.Error(t, err) +} + +func TestMultiSigMigration(t *testing.T) { + msg := []byte{1, 2, 3, 4} + pkSet, sigs := generatePubKeysAndSignatures(2, msg) + multisignature := multisig.NewMultisig(2) + + multisigKey := kmultisig.NewLegacyAminoPubKey(2, pkSet) + signBytesFn := func(mode signing.SignMode) ([]byte, error) { return msg, nil } + + cdc := codec.NewLegacyAmino() + + err := multisig.AddSignatureFromPubKey(multisignature, sigs[0], pkSet[0], pkSet) + + // create a StdSignature for msg, and convert it to sigV2 + sig := authtypes.StdSignature{PubKey: pkSet[1], Signature: msg} + sigV2, err := authtypes.StdSignatureToSignatureV2(cdc, sig) + require.NoError(t, multisig.AddSignatureV2(multisignature, sigV2, pkSet)) + + require.NoError(t, err) + require.NotNil(t, sigV2) + + require.NoError(t, multisigKey.VerifyMultisignature(signBytesFn, multisignature)) +} + +func TestPubKeyMultisigThresholdAminoToIface(t *testing.T) { + msg := []byte{1, 2, 3, 4} + pubkeys, _ := generatePubKeysAndSignatures(5, msg) + multisigKey := kmultisig.NewLegacyAminoPubKey(2, pubkeys) + + ab, err := kmultisig.AminoCdc.MarshalBinaryLengthPrefixed(multisigKey) + require.NoError(t, err) + // like other crypto.Pubkey implementations (e.g. ed25519.PubKeyMultisigThreshold), + // LegacyAminoPubKey should be deserializable into a crypto.LegacyAminoPubKey: + var pubKey kmultisig.LegacyAminoPubKey + err = kmultisig.AminoCdc.UnmarshalBinaryLengthPrefixed(ab, &pubKey) + require.NoError(t, err) + + require.Equal(t, multisigKey.Equals(&pubKey), true) +} + func generatePubKeysAndSignatures(n int, msg []byte) (pubKeys []tmcrypto.PubKey, signatures []signing.SignatureData) { pubKeys = make([]tmcrypto.PubKey, n) signatures = make([]signing.SignatureData, n) @@ -184,7 +300,7 @@ func generatePubKeysAndSignatures(n int, msg []byte) (pubKeys []tmcrypto.PubKey, return } -func generateNestedMultiSignature(n int, msg []byte) (multisig.PubKey, *signing.MultiSignatureData, error) { +func generateNestedMultiSignature(n int, msg []byte) (multisig.PubKey, *signing.MultiSignatureData) { pubKeys := make([]tmcrypto.PubKey, n) signatures := make([]signing.SignatureData, n) bitArray := crypto.NewCompactBitArray(n) @@ -199,32 +315,20 @@ func generateNestedMultiSignature(n int, msg []byte) (multisig.PubKey, *signing. Signatures: nestedSigs, } signatures[i] = nestedSig - anyNestedPks, err := packPubKeys(nestedPks) - if err != nil { - return nil, nil, err - } - pubKeys[i] = &LegacyAminoPubKey{Threshold: 5, PubKeys: anyNestedPks} + pubKeys[i] = kmultisig.NewLegacyAminoPubKey(5, nestedPks) bitArray.SetIndex(i, true) } - anyPubKeys, err := packPubKeys(pubKeys) - if err != nil { - return nil, nil, err - } - return &LegacyAminoPubKey{Threshold: uint32(n), PubKeys: anyPubKeys}, &signing.MultiSignatureData{ + return kmultisig.NewLegacyAminoPubKey(n, pubKeys), &signing.MultiSignatureData{ BitArray: bitArray, Signatures: signatures, - }, nil -} - -func packPubKeys(pubKeys []tmcrypto.PubKey) ([]*types.Any, error) { - anyPubKeys := make([]*types.Any, len(pubKeys)) - - for i := 0; i < len(pubKeys); i++ { - any, err := types.NewAnyWithValue(pubKeys[i].(proto.Message)) - if err != nil { - return nil, err - } - anyPubKeys[i] = any } - return anyPubKeys, nil +} + +func reorderPubKey(pk *kmultisig.LegacyAminoPubKey) (other *kmultisig.LegacyAminoPubKey) { + pubkeysCpy := make([]*types.Any, len(pk.PubKeys)) + copy(pubkeysCpy, pk.PubKeys) + pubkeysCpy[0] = pk.PubKeys[1] + pubkeysCpy[1] = pk.PubKeys[0] + other = &kmultisig.LegacyAminoPubKey{Threshold: 2, PubKeys: pubkeysCpy} + return } diff --git a/crypto/types/multisig/codec.go b/crypto/types/multisig/codec.go deleted file mode 100644 index 8358cb5526..0000000000 --- a/crypto/types/multisig/codec.go +++ /dev/null @@ -1,30 +0,0 @@ -package multisig - -import ( - amino "github.com/tendermint/go-amino" - "github.com/tendermint/tendermint/crypto" - "github.com/tendermint/tendermint/crypto/ed25519" - "github.com/tendermint/tendermint/crypto/sr25519" - - "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" -) - -// TODO: Figure out API for others to either add their own pubkey types, or -// to make verify / marshal accept a Cdc. -const ( - PubKeyAminoRoute = "tendermint/PubKeyMultisigThreshold" -) - -var Cdc = amino.NewCodec() - -func init() { - Cdc.RegisterInterface((*crypto.PubKey)(nil), nil) - Cdc.RegisterConcrete(PubKeyMultisigThreshold{}, - PubKeyAminoRoute, nil) - Cdc.RegisterConcrete(ed25519.PubKey{}, - ed25519.PubKeyName, nil) - Cdc.RegisterConcrete(sr25519.PubKey{}, - sr25519.PubKeyName, nil) - Cdc.RegisterConcrete(&secp256k1.PubKey{}, - secp256k1.PubKeyName, nil) -} diff --git a/crypto/types/multisig/threshold_pubkey.go b/crypto/types/multisig/threshold_pubkey.go deleted file mode 100644 index 7a3c410b7b..0000000000 --- a/crypto/types/multisig/threshold_pubkey.go +++ /dev/null @@ -1,164 +0,0 @@ -package multisig - -import ( - "fmt" - - "github.com/tendermint/tendermint/crypto" - - "github.com/cosmos/cosmos-sdk/types/tx/signing" -) - -// PubKeyMultisigThreshold implements a K of N threshold multisig. -type PubKeyMultisigThreshold struct { - K uint `json:"threshold"` - PubKeys []crypto.PubKey `json:"pubkeys"` -} - -var _ PubKey = PubKeyMultisigThreshold{} - -// NewPubKeyMultisigThreshold returns a new PubKeyMultisigThreshold. -// Panics if len(pubkeys) < k or 0 >= k. -func NewPubKeyMultisigThreshold(k int, pubkeys []crypto.PubKey) PubKey { - if k <= 0 { - panic("threshold k of n multisignature: k <= 0") - } - if len(pubkeys) < k { - panic("threshold k of n multisignature: len(pubkeys) < k") - } - for _, pubkey := range pubkeys { - if pubkey == nil { - panic("nil pubkey") - } - } - return PubKeyMultisigThreshold{uint(k), pubkeys} -} - -// VerifySignature expects sig to be an amino encoded version of a MultiSignature. -// Returns true iff the multisignature contains k or more signatures -// for the correct corresponding keys, -// and all signatures are valid. (Not just k of the signatures) -// The multisig uses a bitarray, so multiple signatures for the same key is not -// a concern. -// -// NOTE: VerifyMultisignature should preferred to VerifySignature which only works -// with amino multisignatures. -func (pk PubKeyMultisigThreshold) VerifySignature(msg []byte, marshalledSig []byte) bool { - var sig AminoMultisignature - err := Cdc.UnmarshalBinaryBare(marshalledSig, &sig) - if err != nil { - return false - } - size := sig.BitArray.Count() - // ensure bit array is the correct size - if len(pk.PubKeys) != size { - return false - } - // ensure size of signature list - if len(sig.Sigs) < int(pk.K) || len(sig.Sigs) > size { - return false - } - // ensure at least k signatures are set - if sig.BitArray.NumTrueBitsBefore(size) < int(pk.K) { - return false - } - // index in the list of signatures which we are concerned with. - sigIndex := 0 - for i := 0; i < size; i++ { - if sig.BitArray.GetIndex(i) { - if !pk.PubKeys[i].VerifySignature(msg, sig.Sigs[sigIndex]) { - return false - } - sigIndex++ - } - } - return true -} - -// VerifyMultisignature implements the PubKey.VerifyMultisignature method -func (pk PubKeyMultisigThreshold) VerifyMultisignature(getSignBytes GetSignBytesFunc, sig *signing.MultiSignatureData) error { - bitarray := sig.BitArray - sigs := sig.Signatures - size := bitarray.Count() - // ensure bit array is the correct size - if len(pk.PubKeys) != size { - return fmt.Errorf("bit array size is incorrect %d", len(pk.PubKeys)) - } - // ensure size of signature list - if len(sigs) < int(pk.K) || len(sigs) > size { - return fmt.Errorf("signature size is incorrect %d", len(sigs)) - } - // ensure at least k signatures are set - if bitarray.NumTrueBitsBefore(size) < int(pk.K) { - return fmt.Errorf("minimum number of signatures not set, have %d, expected %d", bitarray.NumTrueBitsBefore(size), int(pk.K)) - } - // index in the list of signatures which we are concerned with. - sigIndex := 0 - for i := 0; i < size; i++ { - if bitarray.GetIndex(i) { - si := sig.Signatures[sigIndex] - switch si := si.(type) { - case *signing.SingleSignatureData: - msg, err := getSignBytes(si.SignMode) - if err != nil { - return err - } - if !pk.PubKeys[i].VerifySignature(msg, si.Signature) { - return err - } - case *signing.MultiSignatureData: - nestedMultisigPk, ok := pk.PubKeys[i].(PubKey) - if !ok { - return fmt.Errorf("unable to parse pubkey of index %d", i) - } - if err := nestedMultisigPk.VerifyMultisignature(getSignBytes, si); err != nil { - return err - } - default: - return fmt.Errorf("improper signature data type for index %d", sigIndex) - } - sigIndex++ - } - } - return nil -} - -// GetPubKeys implements the PubKey.GetPubKeys method -func (pk PubKeyMultisigThreshold) GetPubKeys() []crypto.PubKey { - return pk.PubKeys -} - -// Bytes returns the amino encoded version of the PubKeyMultisigThreshold -func (pk PubKeyMultisigThreshold) Bytes() []byte { - return Cdc.MustMarshalBinaryBare(pk) -} - -// Address returns tmhash(PubKeyMultisigThreshold.Bytes()) -func (pk PubKeyMultisigThreshold) Address() crypto.Address { - return crypto.AddressHash(pk.Bytes()) -} - -// Equals returns true iff pk and other both have the same number of keys, and -// all constituent keys are the same, and in the same order. -func (pk PubKeyMultisigThreshold) Equals(other crypto.PubKey) bool { - otherKey, sameType := other.(PubKey) - if !sameType { - return false - } - otherPubKeys := otherKey.GetPubKeys() - if pk.GetThreshold() != otherKey.GetThreshold() || len(pk.PubKeys) != len(otherPubKeys) { - return false - } - for i := 0; i < len(pk.PubKeys); i++ { - if !pk.PubKeys[i].Equals(otherPubKeys[i]) { - return false - } - } - return true -} - -// GetThreshold implements the PubKey.GetThreshold method -func (pk PubKeyMultisigThreshold) GetThreshold() uint { - return pk.K -} - -func (pk PubKeyMultisigThreshold) Type() string { return "PubKeyMultisigThreshold" } diff --git a/crypto/types/multisig/threshold_pubkey_test.go b/crypto/types/multisig/threshold_pubkey_test.go deleted file mode 100644 index 4d753675cf..0000000000 --- a/crypto/types/multisig/threshold_pubkey_test.go +++ /dev/null @@ -1,311 +0,0 @@ -package multisig_test - -import ( - "math/rand" - "testing" - - proto "github.com/gogo/protobuf/proto" - "github.com/stretchr/testify/require" - "github.com/tendermint/tendermint/crypto" - "github.com/tendermint/tendermint/crypto/ed25519" - "github.com/tendermint/tendermint/crypto/sr25519" - - "github.com/cosmos/cosmos-sdk/codec" - codectypes "github.com/cosmos/cosmos-sdk/codec/types" - - kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" - "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" - "github.com/cosmos/cosmos-sdk/crypto/types" - "github.com/cosmos/cosmos-sdk/crypto/types/multisig" - "github.com/cosmos/cosmos-sdk/types/tx/signing" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" -) - -// This tests multisig functionality, but it expects the first k signatures to be valid -// TODO: Adapt it to give more flexibility about first k signatures being valid -func TestThresholdMultisigValidCases(t *testing.T) { - pkSet1, sigSet1 := generatePubKeysAndSignatures(5, []byte{1, 2, 3, 4}) - cases := []struct { - msg []byte - k int - pubkeys []crypto.PubKey - signingIndices []int - // signatures should be the same size as signingIndices. - signatures []signing.SignatureData - passAfterKSignatures []bool - }{ - { - msg: []byte{1, 2, 3, 4}, - k: 2, - pubkeys: pkSet1, - signingIndices: []int{0, 3, 1}, - signatures: sigSet1, - passAfterKSignatures: []bool{false}, - }, - } - for tcIndex, tc := range cases { - multisigKey := multisig.NewPubKeyMultisigThreshold(tc.k, tc.pubkeys) - multisignature := multisig.NewMultisig(len(tc.pubkeys)) - signBytesFn := func(mode signing.SignMode) ([]byte, error) { return tc.msg, nil } - - for i := 0; i < tc.k-1; i++ { - signingIndex := tc.signingIndices[i] - require.NoError( - t, - multisig.AddSignatureFromPubKey(multisignature, tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys), - ) - require.Error( - t, - multisigKey.VerifyMultisignature(signBytesFn, multisignature), - "multisig passed when i < k, tc %d, i %d", tcIndex, i, - ) - require.NoError( - t, - multisig.AddSignatureFromPubKey(multisignature, tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys), - ) - require.Equal( - t, - i+1, - len(multisignature.Signatures), - "adding a signature for the same pubkey twice increased signature count by 2, tc %d", tcIndex, - ) - } - require.Error( - t, - multisigKey.VerifyMultisignature(signBytesFn, multisignature), - "multisig passed with k - 1 sigs, tc %d", tcIndex, - ) - require.NoError( - t, - multisig.AddSignatureFromPubKey( - multisignature, - tc.signatures[tc.signingIndices[tc.k]], - tc.pubkeys[tc.signingIndices[tc.k]], - tc.pubkeys, - ), - ) - require.NoError( - t, - multisigKey.VerifyMultisignature(signBytesFn, multisignature), - "multisig failed after k good signatures, tc %d", tcIndex, - ) - - for i := tc.k + 1; i < len(tc.signingIndices); i++ { - signingIndex := tc.signingIndices[i] - - require.NoError( - t, - multisig.AddSignatureFromPubKey(multisignature, tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys), - ) - require.Equal( - t, - tc.passAfterKSignatures[i-(tc.k)-1], - multisigKey.VerifyMultisignature(func(mode signing.SignMode) ([]byte, error) { - return tc.msg, nil - }, multisignature), - "multisig didn't verify as expected after k sigs, tc %d, i %d", tcIndex, i, - ) - require.NoError( - t, - multisig.AddSignatureFromPubKey(multisignature, tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys), - ) - require.Equal( - t, - i+1, - len(multisignature.Signatures), - "adding a signature for the same pubkey twice increased signature count by 2, tc %d", tcIndex, - ) - } - } -} - -// TODO: Fully replace this test with table driven tests -func TestThresholdMultisigDuplicateSignatures(t *testing.T) { - msg := []byte{1, 2, 3, 4, 5} - pubkeys, sigs := generatePubKeysAndSignatures(5, msg) - multisigKey := multisig.NewPubKeyMultisigThreshold(2, pubkeys) - multisignature := multisig.NewMultisig(5) - signBytesFn := func(mode signing.SignMode) ([]byte, error) { return msg, nil } - - require.Error(t, multisigKey.VerifyMultisignature(signBytesFn, multisignature)) - multisig.AddSignatureFromPubKey(multisignature, sigs[0], pubkeys[0], pubkeys) - // Add second signature manually - multisignature.Signatures = append(multisignature.Signatures, sigs[0]) - require.Error(t, multisigKey.VerifyMultisignature(signBytesFn, multisignature)) -} - -func TestMultiSigPubKeyEquality(t *testing.T) { - pubKey1 := secp256k1.GenPrivKey().PubKey() - pubKey2 := secp256k1.GenPrivKey().PubKey() - pubkeys := []crypto.PubKey{pubKey1, pubKey2} - multisigKey := multisig.NewPubKeyMultisigThreshold(2, pubkeys) - var other multisig.PubKey - - testCases := []struct { - msg string - malleate func() - expectEq bool - }{ - { - "equals", - func() { - var otherPubKey multisig.PubKeyMultisigThreshold - multisig.Cdc.MustUnmarshalBinaryBare(multisigKey.Bytes(), &otherPubKey) - other = otherPubKey - }, - true, - }, - { - "ensure that reordering pubkeys is treated as a different pubkey", - func() { - pubkeysCpy := make([]crypto.PubKey, 2) - copy(pubkeysCpy, pubkeys) - pubkeysCpy[0] = pubkeys[1] - pubkeysCpy[1] = pubkeys[0] - other = multisig.NewPubKeyMultisigThreshold(2, pubkeysCpy) - }, - false, - }, - { - "equals with proto pub key", - func() { - anyPubKeys := make([]*codectypes.Any, len(pubkeys)) - - for i := 0; i < len(pubkeys); i++ { - any, err := codectypes.NewAnyWithValue(pubkeys[i].(proto.Message)) - require.NoError(t, err) - anyPubKeys[i] = any - } - other = &kmultisig.LegacyAminoPubKey{Threshold: 2, PubKeys: anyPubKeys} - }, - true, - }, - } - - for _, tc := range testCases { - t.Run(tc.msg, func(t *testing.T) { - tc.malleate() - eq := multisigKey.Equals(other) - require.Equal(t, eq, tc.expectEq) - }) - } -} - -func TestAddress(t *testing.T) { - msg := []byte{1, 2, 3, 4} - pubkeys, _ := generatePubKeysAndSignatures(5, msg) - multisigKey := multisig.NewPubKeyMultisigThreshold(2, pubkeys) - require.Len(t, multisigKey.Address().Bytes(), 20) -} - -func TestPubKeyMultisigThresholdAminoToIface(t *testing.T) { - msg := []byte{1, 2, 3, 4} - pubkeys, _ := generatePubKeysAndSignatures(5, msg) - multisigKey := multisig.NewPubKeyMultisigThreshold(2, pubkeys) - - ab, err := multisig.Cdc.MarshalBinaryLengthPrefixed(multisigKey) - require.NoError(t, err) - // like other crypto.Pubkey implementations (e.g. ed25519.PubKeyMultisigThreshold), - // PubKeyMultisigThreshold should be deserializable into a crypto.PubKeyMultisigThreshold: - var pubKey crypto.PubKey - err = multisig.Cdc.UnmarshalBinaryLengthPrefixed(ab, &pubKey) - require.NoError(t, err) - - require.Equal(t, multisigKey, pubKey) -} - -func TestMultiSignature(t *testing.T) { - msg := []byte{1, 2, 3, 4} - pk, sig := generateNestedMultiSignature(3, msg) - signBytesFn := func(mode signing.SignMode) ([]byte, error) { return msg, nil } - err := pk.VerifyMultisignature(signBytesFn, sig) - require.NoError(t, err) -} - -func TestMultiSigMigration(t *testing.T) { - msg := []byte{1, 2, 3, 4} - pkSet, sigs := generatePubKeysAndSignatures(2, msg) - multisignature := multisig.NewMultisig(2) - - multisigKey := multisig.NewPubKeyMultisigThreshold(2, pkSet) - signBytesFn := func(mode signing.SignMode) ([]byte, error) { return msg, nil } - - cdc := codec.NewLegacyAmino() - - err := multisig.AddSignatureFromPubKey(multisignature, sigs[0], pkSet[0], pkSet) - - // create a StdSignature for msg, and convert it to sigV2 - sig := authtypes.StdSignature{PubKey: pkSet[1], Signature: msg} - sigV2, err := authtypes.StdSignatureToSignatureV2(cdc, sig) - require.NoError(t, multisig.AddSignatureV2(multisignature, sigV2, pkSet)) - - require.NoError(t, err) - require.NotNil(t, sigV2) - - require.NoError(t, multisigKey.VerifyMultisignature(signBytesFn, multisignature)) -} - -func TestAddSignatureFromPubKeyNilCheck(t *testing.T) { - pkSet, sigs := generatePubKeysAndSignatures(5, []byte{1, 2, 3, 4}) - multisignature := multisig.NewMultisig(5) - - //verify no error is returned with all non-nil values - err := multisig.AddSignatureFromPubKey(multisignature, sigs[0], pkSet[0], pkSet) - require.NoError(t, err) - //verify error is returned when key value is nil - err = multisig.AddSignatureFromPubKey(multisignature, sigs[0], pkSet[0], nil) - require.Error(t, err) - //verify error is returned when pubkey value is nil - err = multisig.AddSignatureFromPubKey(multisignature, sigs[0], nil, pkSet) - require.Error(t, err) - //verify error is returned when signature value is nil - err = multisig.AddSignatureFromPubKey(multisignature, nil, pkSet[0], pkSet) - require.Error(t, err) - //verify error is returned when multisignature value is nil - err = multisig.AddSignatureFromPubKey(nil, sigs[0], pkSet[0], pkSet) - require.Error(t, err) -} - -func generatePubKeysAndSignatures(n int, msg []byte) (pubkeys []crypto.PubKey, signatures []signing.SignatureData) { - pubkeys = make([]crypto.PubKey, n) - signatures = make([]signing.SignatureData, n) - for i := 0; i < n; i++ { - var privkey crypto.PrivKey - switch rand.Int63() % 3 { - case 0: - privkey = ed25519.GenPrivKey() - case 1: - privkey = secp256k1.GenPrivKey() - case 2: - privkey = sr25519.GenPrivKey() - } - pubkeys[i] = privkey.PubKey() - sig, _ := privkey.Sign(msg) - signatures[i] = &signing.SingleSignatureData{Signature: sig} - } - return -} - -func generateNestedMultiSignature(n int, msg []byte) (multisig.PubKey, *signing.MultiSignatureData) { - pubkeys := make([]crypto.PubKey, n) - signatures := make([]signing.SignatureData, n) - bitArray := types.NewCompactBitArray(n) - for i := 0; i < n; i++ { - nestedPks, nestedSigs := generatePubKeysAndSignatures(5, msg) - nestedBitArray := types.NewCompactBitArray(5) - for j := 0; j < 5; j++ { - nestedBitArray.SetIndex(j, true) - } - nestedSig := &signing.MultiSignatureData{ - BitArray: nestedBitArray, - Signatures: nestedSigs, - } - signatures[i] = nestedSig - pubkeys[i] = multisig.NewPubKeyMultisigThreshold(5, nestedPks) - bitArray.SetIndex(i, true) - } - return multisig.NewPubKeyMultisigThreshold(n, pubkeys), &signing.MultiSignatureData{ - BitArray: bitArray, - Signatures: signatures, - } -} diff --git a/std/pubkey.go b/std/pubkey.go index 76d5a257a9..53647e537c 100644 --- a/std/pubkey.go +++ b/std/pubkey.go @@ -7,9 +7,9 @@ import ( "github.com/tendermint/tendermint/crypto/ed25519" "github.com/tendermint/tendermint/crypto/sr25519" + kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" "github.com/cosmos/cosmos-sdk/crypto/types" - "github.com/cosmos/cosmos-sdk/crypto/types/multisig" ) // DefaultPublicKeyCodec implements the standard PublicKeyCodec for the SDK which @@ -19,6 +19,7 @@ type DefaultPublicKeyCodec struct{} var _ types.PublicKeyCodec = DefaultPublicKeyCodec{} // Decode implements the PublicKeyCodec.Decode method +// TODO To be removed in https://github.com/cosmos/cosmos-sdk/pull/7276 func (cdc DefaultPublicKeyCodec) Decode(key *types.PublicKey) (crypto.PubKey, error) { // key being nil is allowed as all fields in proto are optional if key == nil { @@ -67,13 +68,15 @@ func (cdc DefaultPublicKeyCodec) Decode(key *types.PublicKey) (crypto.PubKey, er resKeys[i] = dk } - return multisig.NewPubKeyMultisigThreshold(int(key.Multisig.K), resKeys), nil + return kmultisig.NewLegacyAminoPubKey(int(key.Multisig.K), resKeys), nil default: return nil, fmt.Errorf("can't decode PubKey of type %T. Use a custom PublicKeyCodec instead", key) } } // Encode implements the PublicKeyCodec.Encode method +// TODO To be removed in https://github.com/cosmos/cosmos-sdk/pull/7276 + func (cdc DefaultPublicKeyCodec) Encode(key crypto.PubKey) (*types.PublicKey, error) { if key == nil { return &types.PublicKey{}, nil @@ -85,10 +88,11 @@ func (cdc DefaultPublicKeyCodec) Encode(key crypto.PubKey) (*types.PublicKey, er return &types.PublicKey{Sum: &types.PublicKey_Ed25519{Ed25519: key}}, nil case sr25519.PubKey: return &types.PublicKey{Sum: &types.PublicKey_Sr25519{Sr25519: key}}, nil - case multisig.PubKeyMultisigThreshold: + case *kmultisig.LegacyAminoPubKey: pubKeys := key.PubKeys resKeys := make([]*types.PublicKey, len(pubKeys)) - for i, k := range pubKeys { + for i, any := range pubKeys { + k := any.GetCachedValue().(crypto.PubKey) dk, err := cdc.Encode(k) if err != nil { return nil, err @@ -96,7 +100,7 @@ func (cdc DefaultPublicKeyCodec) Encode(key crypto.PubKey) (*types.PublicKey, er resKeys[i] = dk } return &types.PublicKey{Sum: &types.PublicKey_Multisig{Multisig: &types.PubKeyMultisigThreshold{ - K: uint32(key.K), + K: key.Threshold, PubKeys: resKeys, }}}, nil default: diff --git a/std/pubkey_test.go b/std/pubkey_test.go index 302ba22387..cda15459bc 100644 --- a/std/pubkey_test.go +++ b/std/pubkey_test.go @@ -5,11 +5,9 @@ import ( "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/crypto" - "github.com/tendermint/tendermint/crypto/ed25519" - "github.com/tendermint/tendermint/crypto/sr25519" + kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" - "github.com/cosmos/cosmos-sdk/crypto/types/multisig" "github.com/cosmos/cosmos-sdk/std" ) @@ -31,14 +29,8 @@ func TestDefaultPublicKeyCodec(t *testing.T) { pubKeySecp256k1 := secp256k1.GenPrivKey().PubKey() roundTripTest(t, pubKeySecp256k1) - pubKeyEd25519 := ed25519.GenPrivKey().PubKey() - roundTripTest(t, pubKeyEd25519) - - pubKeySr25519 := sr25519.GenPrivKey().PubKey() - roundTripTest(t, pubKeySr25519) - - pubKeyMultisig := multisig.NewPubKeyMultisigThreshold(2, []crypto.PubKey{ - pubKeySecp256k1, pubKeyEd25519, pubKeySr25519, + pubKeyMultisig := kmultisig.NewLegacyAminoPubKey(2, []crypto.PubKey{ + pubKeySecp256k1, secp256k1.GenPrivKey().PubKey(), secp256k1.GenPrivKey().PubKey(), }) roundTripTest(t, pubKeyMultisig) } diff --git a/x/auth/ante/ante_test.go b/x/auth/ante/ante_test.go index 84602abcb9..5a49ebfe07 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -11,8 +11,8 @@ import ( "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/ed25519" + kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" - "github.com/cosmos/cosmos-sdk/crypto/types/multisig" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -935,10 +935,10 @@ func TestCountSubkeys(t *testing.T) { return ret } singleKey := secp256k1.GenPrivKey().PubKey() - singleLevelMultiKey := multisig.NewPubKeyMultisigThreshold(4, genPubKeys(5)) - multiLevelSubKey1 := multisig.NewPubKeyMultisigThreshold(4, genPubKeys(5)) - multiLevelSubKey2 := multisig.NewPubKeyMultisigThreshold(4, genPubKeys(5)) - multiLevelMultiKey := multisig.NewPubKeyMultisigThreshold(2, []crypto.PubKey{ + singleLevelMultiKey := kmultisig.NewLegacyAminoPubKey(4, genPubKeys(5)) + multiLevelSubKey1 := kmultisig.NewLegacyAminoPubKey(4, genPubKeys(5)) + multiLevelSubKey2 := kmultisig.NewLegacyAminoPubKey(4, genPubKeys(5)) + multiLevelMultiKey := kmultisig.NewLegacyAminoPubKey(2, []crypto.PubKey{ multiLevelSubKey1, multiLevelSubKey2, secp256k1.GenPrivKey().PubKey()}) type args struct { pub crypto.PubKey diff --git a/x/auth/ante/basic.go b/x/auth/ante/basic.go index df874a63b1..b73b95656f 100644 --- a/x/auth/ante/basic.go +++ b/x/auth/ante/basic.go @@ -4,7 +4,7 @@ import ( "github.com/tendermint/tendermint/crypto" "github.com/cosmos/cosmos-sdk/codec/legacy" - "github.com/cosmos/cosmos-sdk/crypto/types/multisig" + "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/types/tx/signing" @@ -136,7 +136,7 @@ func (cgts ConsumeTxSizeGasDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim // If the pubkey is a multi-signature pubkey, then we estimate for the maximum // number of signers. - if _, ok := pubkey.(multisig.PubKeyMultisigThreshold); ok { + if _, ok := pubkey.(*multisig.LegacyAminoPubKey); ok { cost *= params.TxSigLimit } diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index 54554f5b95..266951081e 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -8,6 +8,7 @@ import ( "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" + kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" "github.com/cosmos/cosmos-sdk/crypto/types/multisig" "github.com/cosmos/cosmos-sdk/simapp" @@ -70,7 +71,7 @@ func (suite *AnteTestSuite) TestConsumeSignatureVerificationGas() { _, cdc := simapp.MakeCodecs() pkSet1, sigSet1 := generatePubKeysAndSignatures(5, msg, false) - multisigKey1 := multisig.NewPubKeyMultisigThreshold(2, pkSet1) + multisigKey1 := kmultisig.NewLegacyAminoPubKey(2, pkSet1) multisignature1 := multisig.NewMultisig(len(pkSet1)) expectedCost1 := expectedGasCostByKeys(pkSet1) for i := 0; i < len(pkSet1); i++ { diff --git a/x/auth/client/cli/cli_test.go b/x/auth/client/cli/cli_test.go index 0ab22d3f22..ce3edb994f 100644 --- a/x/auth/client/cli/cli_test.go +++ b/x/auth/client/cli/cli_test.go @@ -21,7 +21,7 @@ import ( "github.com/cosmos/cosmos-sdk/codec/types" "github.com/cosmos/cosmos-sdk/crypto/hd" "github.com/cosmos/cosmos-sdk/crypto/keyring" - "github.com/cosmos/cosmos-sdk/crypto/types/multisig" + kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/simapp" "github.com/cosmos/cosmos-sdk/testutil" clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli" @@ -61,7 +61,7 @@ func (s *IntegrationTestSuite) SetupSuite() { account2, _, err := kb.NewMnemonic("newAccount2", keyring.English, sdk.FullFundraiserPath, hd.Secp256k1) s.Require().NoError(err) - multi := multisig.NewPubKeyMultisigThreshold(2, []tmcrypto.PubKey{account1.GetPubKey(), account2.GetPubKey()}) + multi := kmultisig.NewLegacyAminoPubKey(2, []tmcrypto.PubKey{account1.GetPubKey(), account2.GetPubKey()}) _, err = kb.SaveMultisig("multi", multi) s.Require().NoError(err) diff --git a/x/auth/client/cli/tx_multisign.go b/x/auth/client/cli/tx_multisign.go index a7c866f616..e3ef24b42e 100644 --- a/x/auth/client/cli/tx_multisign.go +++ b/x/auth/client/cli/tx_multisign.go @@ -13,6 +13,7 @@ import ( "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/client/tx" "github.com/cosmos/cosmos-sdk/crypto/keyring" + kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/types/multisig" sdk "github.com/cosmos/cosmos-sdk/types" signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" @@ -97,7 +98,7 @@ func makeMultiSignCmd() func(cmd *cobra.Command, args []string) error { return fmt.Errorf("%q must be of type %s: %s", args[1], keyring.TypeMulti, multisigInfo.GetType()) } - multisigPub := multisigInfo.GetPubKey().(multisig.PubKeyMultisigThreshold) + multisigPub := multisigInfo.GetPubKey().(*kmultisig.LegacyAminoPubKey) multisigSig := multisig.NewMultisig(len(multisigPub.PubKeys)) if !clientCtx.Offline { accnum, seq, err := clientCtx.AccountRetriever.GetAccountNumberSequence(clientCtx, multisigInfo.GetAddress()) @@ -127,7 +128,7 @@ func makeMultiSignCmd() func(cmd *cobra.Command, args []string) error { return fmt.Errorf("couldn't verify signature: %w", err) } - if err := multisig.AddSignatureV2(multisigSig, sig, multisigPub.PubKeys); err != nil { + if err := multisig.AddSignatureV2(multisigSig, sig, multisigPub.GetPubKeys()); err != nil { return err } } diff --git a/x/auth/signing/verify_test.go b/x/auth/signing/verify_test.go index 198536111a..c5bc0be316 100644 --- a/x/auth/signing/verify_test.go +++ b/x/auth/signing/verify_test.go @@ -8,6 +8,7 @@ import ( tmproto "github.com/tendermint/tendermint/proto/tendermint/types" "github.com/cosmos/cosmos-sdk/codec" + kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/types/multisig" "github.com/cosmos/cosmos-sdk/simapp" "github.com/cosmos/cosmos-sdk/testutil/testdata" @@ -64,7 +65,7 @@ func TestVerifySignature(t *testing.T) { require.NoError(t, err) pkSet := []crypto.PubKey{pubKey, pubKey1} - multisigKey := multisig.NewPubKeyMultisigThreshold(2, pkSet) + multisigKey := kmultisig.NewLegacyAminoPubKey(2, pkSet) multisignature := multisig.NewMultisig(2) msgs = []sdk.Msg{testdata.NewTestMsg(addr, addr1)} multiSignBytes := types.StdSignBytes(signerData.ChainID, signerData.AccountNumber, signerData.Sequence, 10, fee, msgs, memo) diff --git a/x/auth/testutil/suite.go b/x/auth/testutil/suite.go index 3f6e138e5a..cafc57884f 100644 --- a/x/auth/testutil/suite.go +++ b/x/auth/testutil/suite.go @@ -7,13 +7,12 @@ import ( "github.com/tendermint/tendermint/crypto" "github.com/cosmos/cosmos-sdk/client" + kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/types/multisig" "github.com/cosmos/cosmos-sdk/testutil/testdata" - "github.com/cosmos/cosmos-sdk/x/auth/signing" - - signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" - sdk "github.com/cosmos/cosmos-sdk/types" + signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" + "github.com/cosmos/cosmos-sdk/x/auth/signing" ) // TxConfigTestSuite provides a test suite that can be used to test that a TxConfig implementation is correct @@ -82,7 +81,7 @@ func (s *TxConfigTestSuite) TestTxBuilderSetMsgs() { func (s *TxConfigTestSuite) TestTxBuilderSetSignatures() { privKey, pubkey, addr := testdata.KeyTestPubAddr() privKey2, pubkey2, _ := testdata.KeyTestPubAddr() - multisigPk := multisig.NewPubKeyMultisigThreshold(2, []crypto.PubKey{pubkey, pubkey2}) + multisigPk := kmultisig.NewLegacyAminoPubKey(2, []crypto.PubKey{pubkey, pubkey2}) txBuilder := s.TxConfig.NewTxBuilder() diff --git a/x/auth/types/stdtx.go b/x/auth/types/stdtx.go index cdd9b01b41..214847ee93 100644 --- a/x/auth/types/stdtx.go +++ b/x/auth/types/stdtx.go @@ -12,6 +12,7 @@ import ( "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/codec/legacy" codectypes "github.com/cosmos/cosmos-sdk/codec/types" + kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/types/multisig" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -116,13 +117,13 @@ func (ss StdSignature) MarshalYAML() (interface{}, error) { // CountSubKeys counts the total number of keys for a multi-sig public key. func CountSubKeys(pub crypto.PubKey) int { - v, ok := pub.(multisig.PubKeyMultisigThreshold) + v, ok := pub.(*kmultisig.LegacyAminoPubKey) if !ok { return 1 } numKeys := 0 - for _, subkey := range v.PubKeys { + for _, subkey := range v.GetPubKeys() { numKeys += CountSubKeys(subkey) } diff --git a/x/auth/types/stdtx_test.go b/x/auth/types/stdtx_test.go index 9f01c39fd3..86bf72441d 100644 --- a/x/auth/types/stdtx_test.go +++ b/x/auth/types/stdtx_test.go @@ -13,8 +13,8 @@ import ( "github.com/cosmos/cosmos-sdk/codec" cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec" + kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig" "github.com/cosmos/cosmos-sdk/crypto/types" - "github.com/cosmos/cosmos-sdk/crypto/types/multisig" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -225,7 +225,7 @@ func TestSignatureV2Conversions(t *testing.T) { // multisigs _, pubKey2, _ := testdata.KeyTestPubAddr() - multiPK := multisig.NewPubKeyMultisigThreshold(1, []crypto.PubKey{ + multiPK := kmultisig.NewLegacyAminoPubKey(1, []crypto.PubKey{ pubKey, pubKey2, }) dummy2 := []byte("dummySig2")