From 38b401657e4ad7a7eeb3c30a3cbf512037df3740 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Tue, 28 Aug 2018 00:41:40 -0400 Subject: [PATCH] Cleanup up Multisig naming (#2255) * crypto/multisig: Pubkey -> PubKey * crypto/encoding/amino: use pkg vars for routes * crypto/multisig/bitarray * crypto/multisig: ThresholdMultiSignaturePubKey -> PubKeyMultisigThreshold * crypto/encoding/amino: add PubKeyMultisig to table * remove bA import alias https://github.com/tendermint/tendermint/pull/2255#discussion_r211900709 --- crypto/encoding/amino/amino.go | 12 ++++-- crypto/encoding/amino/encode_test.go | 1 + .../{ => bitarray}/compact_bit_array.go | 2 +- .../{ => bitarray}/compact_bit_array_test.go | 2 +- crypto/multisig/multisignature.go | 9 +++-- ...eshold_multisig.go => threshold_pubkey.go} | 38 +++++++++---------- ...tisig_test.go => threshold_pubkey_test.go} | 24 ++++++------ crypto/multisig/wire.go | 6 +-- 8 files changed, 50 insertions(+), 44 deletions(-) rename crypto/multisig/{ => bitarray}/compact_bit_array.go (99%) rename crypto/multisig/{ => bitarray}/compact_bit_array_test.go (99%) rename crypto/multisig/{threshold_multisig.go => threshold_pubkey.go} (58%) rename crypto/multisig/{threshold_multisig_test.go => threshold_pubkey_test.go} (83%) diff --git a/crypto/encoding/amino/amino.go b/crypto/encoding/amino/amino.go index fd9a08442..7728e6afb 100644 --- a/crypto/encoding/amino/amino.go +++ b/crypto/encoding/amino/amino.go @@ -2,8 +2,10 @@ package cryptoAmino import ( amino "github.com/tendermint/go-amino" + "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/ed25519" + "github.com/tendermint/tendermint/crypto/multisig" "github.com/tendermint/tendermint/crypto/secp256k1" ) @@ -24,15 +26,17 @@ func RegisterAmino(cdc *amino.Codec) { // These are all written here instead of cdc.RegisterInterface((*crypto.PubKey)(nil), nil) cdc.RegisterConcrete(ed25519.PubKeyEd25519{}, - "tendermint/PubKeyEd25519", nil) + ed25519.PubKeyAminoRoute, nil) cdc.RegisterConcrete(secp256k1.PubKeySecp256k1{}, - "tendermint/PubKeySecp256k1", nil) + secp256k1.PubKeyAminoRoute, nil) + cdc.RegisterConcrete(multisig.PubKeyMultisigThreshold{}, + multisig.PubKeyMultisigThresholdAminoRoute, nil) cdc.RegisterInterface((*crypto.PrivKey)(nil), nil) cdc.RegisterConcrete(ed25519.PrivKeyEd25519{}, - "tendermint/PrivKeyEd25519", nil) + ed25519.PrivKeyAminoRoute, nil) cdc.RegisterConcrete(secp256k1.PrivKeySecp256k1{}, - "tendermint/PrivKeySecp256k1", nil) + secp256k1.PrivKeyAminoRoute, nil) } func PrivKeyFromBytes(privKeyBytes []byte) (privKey crypto.PrivKey, err error) { diff --git a/crypto/encoding/amino/encode_test.go b/crypto/encoding/amino/encode_test.go index 0581ba643..ea9e9809f 100644 --- a/crypto/encoding/amino/encode_test.go +++ b/crypto/encoding/amino/encode_test.go @@ -53,6 +53,7 @@ func ExamplePrintRegisteredTypes() { //| ---- | ---- | ------ | ----- | ------ | //| PubKeyEd25519 | tendermint/PubKeyEd25519 | 0x1624DE64 | 0x20 | | //| PubKeySecp256k1 | tendermint/PubKeySecp256k1 | 0xEB5AE987 | 0x21 | | + //| PubKeyMultisigThreshold | tendermint/PubKeyMultisigThreshold | 0x22C1F7E2 | variable | | //| PrivKeyEd25519 | tendermint/PrivKeyEd25519 | 0xA3288910 | 0x40 | | //| PrivKeySecp256k1 | tendermint/PrivKeySecp256k1 | 0xE1B0F79B | 0x20 | | } diff --git a/crypto/multisig/compact_bit_array.go b/crypto/multisig/bitarray/compact_bit_array.go similarity index 99% rename from crypto/multisig/compact_bit_array.go rename to crypto/multisig/bitarray/compact_bit_array.go index 94e9791cd..0152db724 100644 --- a/crypto/multisig/compact_bit_array.go +++ b/crypto/multisig/bitarray/compact_bit_array.go @@ -1,4 +1,4 @@ -package multisig +package bitarray import ( "bytes" diff --git a/crypto/multisig/compact_bit_array_test.go b/crypto/multisig/bitarray/compact_bit_array_test.go similarity index 99% rename from crypto/multisig/compact_bit_array_test.go rename to crypto/multisig/bitarray/compact_bit_array_test.go index 4684ba23f..4612ae25a 100644 --- a/crypto/multisig/compact_bit_array_test.go +++ b/crypto/multisig/bitarray/compact_bit_array_test.go @@ -1,4 +1,4 @@ -package multisig +package bitarray import ( "encoding/json" diff --git a/crypto/multisig/multisignature.go b/crypto/multisig/multisignature.go index 29e8a30b9..0d1796890 100644 --- a/crypto/multisig/multisignature.go +++ b/crypto/multisig/multisignature.go @@ -4,12 +4,13 @@ import ( "errors" "github.com/tendermint/tendermint/crypto" + "github.com/tendermint/tendermint/crypto/multisig/bitarray" ) // Multisignature is used to represent the signature object used in the multisigs. // Sigs is a list of signatures, sorted by corresponding index. type Multisignature struct { - BitArray *CompactBitArray + BitArray *bitarray.CompactBitArray Sigs [][]byte } @@ -17,7 +18,7 @@ type Multisignature struct { func NewMultisig(n int) *Multisignature { // Default the signature list to have a capacity of two, since we can // expect that most multisigs will require multiple signers. - return &Multisignature{NewCompactBitArray(n), make([][]byte, 0, 2)} + return &Multisignature{bitarray.NewCompactBitArray(n), make([][]byte, 0, 2)} } // GetIndex returns the index of pk in keys. Returns -1 if not found @@ -52,9 +53,9 @@ func (mSig *Multisignature) AddSignature(sig []byte, index int) { mSig.Sigs[newSigIndex] = sig } -// AddSignatureFromPubkey adds a signature to the multisig, +// AddSignatureFromPubKey adds a signature to the multisig, // at the index in keys corresponding to the provided pubkey. -func (mSig *Multisignature) AddSignatureFromPubkey(sig []byte, pubkey crypto.PubKey, keys []crypto.PubKey) error { +func (mSig *Multisignature) AddSignatureFromPubKey(sig []byte, pubkey crypto.PubKey, keys []crypto.PubKey) error { index := getIndex(pubkey, keys) if index == -1 { return errors.New("provided key didn't exist in pubkeys") diff --git a/crypto/multisig/threshold_multisig.go b/crypto/multisig/threshold_pubkey.go similarity index 58% rename from crypto/multisig/threshold_multisig.go rename to crypto/multisig/threshold_pubkey.go index 58a3ea0e3..ca8d42303 100644 --- a/crypto/multisig/threshold_multisig.go +++ b/crypto/multisig/threshold_pubkey.go @@ -5,24 +5,24 @@ import ( "github.com/tendermint/tendermint/crypto/tmhash" ) -// ThresholdMultiSignaturePubKey implements a K of N threshold multisig. -type ThresholdMultiSignaturePubKey struct { +// PubKeyMultisigThreshold implements a K of N threshold multisig. +type PubKeyMultisigThreshold struct { K uint `json:"threshold"` - Pubkeys []crypto.PubKey `json:"pubkeys"` + PubKeys []crypto.PubKey `json:"pubkeys"` } -var _ crypto.PubKey = &ThresholdMultiSignaturePubKey{} +var _ crypto.PubKey = &PubKeyMultisigThreshold{} -// NewThresholdMultiSignaturePubKey returns a new ThresholdMultiSignaturePubKey. +// NewPubKeyMultisigThreshold returns a new PubKeyMultisigThreshold. // Panics if len(pubkeys) < k or 0 >= k. -func NewThresholdMultiSignaturePubKey(k int, pubkeys []crypto.PubKey) crypto.PubKey { +func NewPubKeyMultisigThreshold(k int, pubkeys []crypto.PubKey) crypto.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") } - return &ThresholdMultiSignaturePubKey{uint(k), pubkeys} + return &PubKeyMultisigThreshold{uint(k), pubkeys} } // VerifyBytes expects sig to be an amino encoded version of a MultiSignature. @@ -31,7 +31,7 @@ func NewThresholdMultiSignaturePubKey(k int, pubkeys []crypto.PubKey) crypto.Pub // 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. -func (pk *ThresholdMultiSignaturePubKey) VerifyBytes(msg []byte, marshalledSig []byte) bool { +func (pk *PubKeyMultisigThreshold) VerifyBytes(msg []byte, marshalledSig []byte) bool { var sig *Multisignature err := cdc.UnmarshalBinaryBare(marshalledSig, &sig) if err != nil { @@ -39,7 +39,7 @@ func (pk *ThresholdMultiSignaturePubKey) VerifyBytes(msg []byte, marshalledSig [ } size := sig.BitArray.Size() // ensure bit array is the correct size - if len(pk.Pubkeys) != size { + if len(pk.PubKeys) != size { return false } // ensure size of signature list @@ -54,7 +54,7 @@ func (pk *ThresholdMultiSignaturePubKey) VerifyBytes(msg []byte, marshalledSig [ sigIndex := 0 for i := 0; i < size; i++ { if sig.BitArray.GetIndex(i) { - if !pk.Pubkeys[i].VerifyBytes(msg, sig.Sigs[sigIndex]) { + if !pk.PubKeys[i].VerifyBytes(msg, sig.Sigs[sigIndex]) { return false } sigIndex++ @@ -63,28 +63,28 @@ func (pk *ThresholdMultiSignaturePubKey) VerifyBytes(msg []byte, marshalledSig [ return true } -// Bytes returns the amino encoded version of the ThresholdMultiSignaturePubKey -func (pk *ThresholdMultiSignaturePubKey) Bytes() []byte { +// Bytes returns the amino encoded version of the PubKeyMultisigThreshold +func (pk *PubKeyMultisigThreshold) Bytes() []byte { return cdc.MustMarshalBinaryBare(pk) } -// Address returns tmhash(ThresholdMultiSignaturePubKey.Bytes()) -func (pk *ThresholdMultiSignaturePubKey) Address() crypto.Address { +// Address returns tmhash(PubKeyMultisigThreshold.Bytes()) +func (pk *PubKeyMultisigThreshold) Address() crypto.Address { return crypto.Address(tmhash.Sum(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 *ThresholdMultiSignaturePubKey) Equals(other crypto.PubKey) bool { - otherKey, sameType := other.(*ThresholdMultiSignaturePubKey) +func (pk *PubKeyMultisigThreshold) Equals(other crypto.PubKey) bool { + otherKey, sameType := other.(*PubKeyMultisigThreshold) if !sameType { return false } - if pk.K != otherKey.K || len(pk.Pubkeys) != len(otherKey.Pubkeys) { + if pk.K != otherKey.K || len(pk.PubKeys) != len(otherKey.PubKeys) { return false } - for i := 0; i < len(pk.Pubkeys); i++ { - if !pk.Pubkeys[i].Equals(otherKey.Pubkeys[i]) { + for i := 0; i < len(pk.PubKeys); i++ { + if !pk.PubKeys[i].Equals(otherKey.PubKeys[i]) { return false } } diff --git a/crypto/multisig/threshold_multisig_test.go b/crypto/multisig/threshold_pubkey_test.go similarity index 83% rename from crypto/multisig/threshold_multisig_test.go rename to crypto/multisig/threshold_pubkey_test.go index 2d2fdeec8..bfc874ebe 100644 --- a/crypto/multisig/threshold_multisig_test.go +++ b/crypto/multisig/threshold_pubkey_test.go @@ -34,30 +34,30 @@ func TestThresholdMultisigValidCases(t *testing.T) { }, } for tcIndex, tc := range cases { - multisigKey := NewThresholdMultiSignaturePubKey(tc.k, tc.pubkeys) + multisigKey := NewPubKeyMultisigThreshold(tc.k, tc.pubkeys) multisignature := NewMultisig(len(tc.pubkeys)) for i := 0; i < tc.k-1; i++ { signingIndex := tc.signingIndices[i] - multisignature.AddSignatureFromPubkey(tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys) + multisignature.AddSignatureFromPubKey(tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys) require.False(t, multisigKey.VerifyBytes(tc.msg, multisignature.Marshal()), "multisig passed when i < k, tc %d, i %d", tcIndex, i) - multisignature.AddSignatureFromPubkey(tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys) + multisignature.AddSignatureFromPubKey(tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys) require.Equal(t, i+1, len(multisignature.Sigs), "adding a signature for the same pubkey twice increased signature count by 2, tc %d", tcIndex) } require.False(t, multisigKey.VerifyBytes(tc.msg, multisignature.Marshal()), "multisig passed with k - 1 sigs, tc %d", tcIndex) - multisignature.AddSignatureFromPubkey(tc.signatures[tc.signingIndices[tc.k]], tc.pubkeys[tc.signingIndices[tc.k]], tc.pubkeys) + multisignature.AddSignatureFromPubKey(tc.signatures[tc.signingIndices[tc.k]], tc.pubkeys[tc.signingIndices[tc.k]], tc.pubkeys) require.True(t, multisigKey.VerifyBytes(tc.msg, multisignature.Marshal()), "multisig failed after k good signatures, tc %d", tcIndex) for i := tc.k + 1; i < len(tc.signingIndices); i++ { signingIndex := tc.signingIndices[i] - multisignature.AddSignatureFromPubkey(tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys) + multisignature.AddSignatureFromPubKey(tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys) require.Equal(t, tc.passAfterKSignatures[i-tc.k-1], multisigKey.VerifyBytes(tc.msg, multisignature.Marshal()), "multisig didn't verify as expected after k sigs, tc %d, i %d", tcIndex, i) - multisignature.AddSignatureFromPubkey(tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys) + multisignature.AddSignatureFromPubKey(tc.signatures[signingIndex], tc.pubkeys[signingIndex], tc.pubkeys) require.Equal(t, i+1, len(multisignature.Sigs), "adding a signature for the same pubkey twice increased signature count by 2, tc %d", tcIndex) } @@ -68,21 +68,21 @@ func TestThresholdMultisigValidCases(t *testing.T) { func TestThresholdMultisigDuplicateSignatures(t *testing.T) { msg := []byte{1, 2, 3, 4, 5} pubkeys, sigs := generatePubKeysAndSignatures(5, msg) - multisigKey := NewThresholdMultiSignaturePubKey(2, pubkeys) + multisigKey := NewPubKeyMultisigThreshold(2, pubkeys) multisignature := NewMultisig(5) require.False(t, multisigKey.VerifyBytes(msg, multisignature.Marshal())) - multisignature.AddSignatureFromPubkey(sigs[0], pubkeys[0], pubkeys) + multisignature.AddSignatureFromPubKey(sigs[0], pubkeys[0], pubkeys) // Add second signature manually multisignature.Sigs = append(multisignature.Sigs, sigs[0]) require.False(t, multisigKey.VerifyBytes(msg, multisignature.Marshal())) } // TODO: Fully replace this test with table driven tests -func TestMultiSigPubkeyEquality(t *testing.T) { +func TestMultiSigPubKeyEquality(t *testing.T) { msg := []byte{1, 2, 3, 4} pubkeys, _ := generatePubKeysAndSignatures(5, msg) - multisigKey := NewThresholdMultiSignaturePubKey(2, pubkeys) - var unmarshalledMultisig *ThresholdMultiSignaturePubKey + multisigKey := NewPubKeyMultisigThreshold(2, pubkeys) + var unmarshalledMultisig *PubKeyMultisigThreshold cdc.MustUnmarshalBinaryBare(multisigKey.Bytes(), &unmarshalledMultisig) require.True(t, multisigKey.Equals(unmarshalledMultisig)) @@ -91,7 +91,7 @@ func TestMultiSigPubkeyEquality(t *testing.T) { copy(pubkeysCpy, pubkeys) pubkeysCpy[4] = pubkeys[3] pubkeysCpy[3] = pubkeys[4] - multisigKey2 := NewThresholdMultiSignaturePubKey(2, pubkeysCpy) + multisigKey2 := NewPubKeyMultisigThreshold(2, pubkeysCpy) require.False(t, multisigKey.Equals(multisigKey2)) } diff --git a/crypto/multisig/wire.go b/crypto/multisig/wire.go index 4c48c6649..68b84fbfb 100644 --- a/crypto/multisig/wire.go +++ b/crypto/multisig/wire.go @@ -10,15 +10,15 @@ import ( // TODO: Figure out API for others to either add their own pubkey types, or // to make verify / marshal accept a cdc. const ( - ThresholdPubkeyAminoRoute = "tendermint/PubkeyMultisigThreshold" + PubKeyMultisigThresholdAminoRoute = "tendermint/PubKeyMultisigThreshold" ) var cdc = amino.NewCodec() func init() { cdc.RegisterInterface((*crypto.PubKey)(nil), nil) - cdc.RegisterConcrete(ThresholdMultiSignaturePubKey{}, - ThresholdPubkeyAminoRoute, nil) + cdc.RegisterConcrete(PubKeyMultisigThreshold{}, + PubKeyMultisigThresholdAminoRoute, nil) cdc.RegisterConcrete(ed25519.PubKeyEd25519{}, ed25519.PubKeyAminoRoute, nil) cdc.RegisterConcrete(secp256k1.PubKeySecp256k1{},