From 96fdec0fcadd2260277f16ad7d96ed342d6110d1 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Thu, 2 Aug 2018 23:18:09 -0700 Subject: [PATCH 01/10] crypto: Add compact bit array for intended usage in the multisig This is in a separate PR for ease of review. --- crypto/multisig/compact_bit_array.go | 218 ++++++++++++++++++++++ crypto/multisig/compact_bit_array_test.go | 169 +++++++++++++++++ 2 files changed, 387 insertions(+) create mode 100644 crypto/multisig/compact_bit_array.go create mode 100644 crypto/multisig/compact_bit_array_test.go diff --git a/crypto/multisig/compact_bit_array.go b/crypto/multisig/compact_bit_array.go new file mode 100644 index 000000000..d14dd8e67 --- /dev/null +++ b/crypto/multisig/compact_bit_array.go @@ -0,0 +1,218 @@ +package multisig + +import ( + "bytes" + "encoding/binary" + "errors" + "fmt" + "regexp" + "strings" +) + +// CompactBitArray is an implementation of a space efficient bit array. +// This is used to ensure that the encoded data takes up a minimal amount of +// space after amino encoding. +// This is not thread safe, and is not intended for concurrent usage. +type CompactBitArray struct { + ExtraBitsStored byte `json:"extra_bits"` // The number of extra bits in elems. + Elems []byte `json:"bits"` +} + +// NewCompactBitArray returns a new compact bit array. +// It returns nil if the number of bits is zero. +func NewCompactBitArray(bits int) *CompactBitArray { + if bits <= 0 { + return nil + } + return &CompactBitArray{ + ExtraBitsStored: byte(bits % 8), + Elems: make([]byte, (bits+7)/8), + } +} + +// Size returns the number of bits in the bitarray +func (bA *CompactBitArray) Size() int { + if bA == nil { + return 0 + } else if bA.ExtraBitsStored == byte(0) { + return len(bA.Elems) * 8 + } + return (len(bA.Elems)-1)*8 + int(bA.ExtraBitsStored) +} + +// GetIndex returns the bit at index i within the bit array. +// The behavior is undefined if i >= bA.Size() +func (bA *CompactBitArray) GetIndex(i int) bool { + if bA == nil { + return false + } + if i >= bA.Size() { + return false + } + return bA.Elems[i>>3]&(uint8(1)< 0 +} + +// SetIndex sets the bit at index i within the bit array. +// The behavior is undefined if i >= bA.Size() +func (bA *CompactBitArray) SetIndex(i int, v bool) bool { + if bA == nil { + return false + } + if i >= bA.Size() { + return false + } + if v { + bA.Elems[i>>3] |= (uint8(1) << uint8(7-(i%8))) + } else { + bA.Elems[i>>3] &= ^(uint8(1) << uint8(7-(i%8))) + } + return true +} + +// Copy returns a copy of the provided bit array. +func (bA *CompactBitArray) Copy() *CompactBitArray { + if bA == nil { + return nil + } + c := make([]byte, len(bA.Elems)) + copy(c, bA.Elems) + return &CompactBitArray{ + ExtraBitsStored: bA.ExtraBitsStored, + Elems: c, + } +} + +// String returns a string representation of CompactBitArray: BA{}, +// where is a sequence of 'x' (1) and '_' (0). +// The includes spaces and newlines to help people. +// For a simple sequence of 'x' and '_' characters with no spaces or newlines, +// see the MarshalJSON() method. +// Example: "BA{_x_}" or "nil-BitArray" for nil. +func (bA *CompactBitArray) String() string { + return bA.StringIndented("") +} + +// StringIndented returns the same thing as String(), but applies the indent +// at every 10th bit, and twice at every 50th bit. +func (bA *CompactBitArray) StringIndented(indent string) string { + if bA == nil { + return "nil-BitArray" + } + lines := []string{} + bits := "" + size := bA.Size() + for i := 0; i < size; i++ { + if bA.GetIndex(i) { + bits += "x" + } else { + bits += "_" + } + if i%100 == 99 { + lines = append(lines, bits) + bits = "" + } + if i%10 == 9 { + bits += indent + } + if i%50 == 49 { + bits += indent + } + } + if len(bits) > 0 { + lines = append(lines, bits) + } + return fmt.Sprintf("BA{%v:%v}", size, strings.Join(lines, indent)) +} + +// MarshalJSON implements json.Marshaler interface by marshaling bit array +// using a custom format: a string of '-' or 'x' where 'x' denotes the 1 bit. +func (bA *CompactBitArray) MarshalJSON() ([]byte, error) { + if bA == nil { + return []byte("null"), nil + } + + bits := `"` + size := bA.Size() + for i := 0; i < size; i++ { + if bA.GetIndex(i) { + bits += `x` + } else { + bits += `_` + } + } + bits += `"` + return []byte(bits), nil +} + +var bitArrayJSONRegexp = regexp.MustCompile(`\A"([_x]*)"\z`) + +// UnmarshalJSON implements json.Unmarshaler interface by unmarshaling a custom +// JSON description. +func (bA *CompactBitArray) UnmarshalJSON(bz []byte) error { + b := string(bz) + if b == "null" { + // This is required e.g. for encoding/json when decoding + // into a pointer with pre-allocated BitArray. + bA.ExtraBitsStored = 0 + bA.Elems = nil + return nil + } + + // Validate 'b'. + match := bitArrayJSONRegexp.FindStringSubmatch(b) + if match == nil { + return fmt.Errorf("BitArray in JSON should be a string of format %q but got %s", bitArrayJSONRegexp.String(), b) + } + bits := match[1] + + // Construct new CompactBitArray and copy over. + numBits := len(bits) + bA2 := NewCompactBitArray(numBits) + for i := 0; i < numBits; i++ { + if bits[i] == 'x' { + bA2.SetIndex(i, true) + } + } + *bA = *bA2 + return nil +} + +// CompactMarshal is a space efficient encoding for CompactBitArray. +// It is not amino compatible. +func (bA *CompactBitArray) CompactMarshal() []byte { + size := bA.Size() + if size <= 0 { + return []byte("null") + } + bz := make([]byte, 0, size/8) + // length prefix number of bits, not number of bytes. This difference + // takes 3-4 bits in encoding, as opposed to instead encoding the number of + // bytes (saving 3-4 bits) and including the offset as a full byte. + bz = appendUvarint(bz, uint64(size)) + bz = append(bz, bA.Elems...) + return bz +} + +// CompactUnmarshal is a space efficient decoding for CompactBitArray. +// It is not amino compatible. +func CompactUnmarshal(bz []byte) (*CompactBitArray, error) { + if len(bz) < 2 { + return nil, errors.New("compact bit array: invalid compact unmarshal size") + } else if bytes.Equal(bz, []byte("null")) { + return NewCompactBitArray(0), nil + } + size, n := binary.Uvarint(bz) + bz = bz[n:] + if len(bz) != int(size+7)/8 { + return nil, errors.New("compact bit array: invalid compact unmarshal size") + } + + bA := &CompactBitArray{byte(int(size % 8)), bz} + return bA, nil +} + +func appendUvarint(b []byte, x uint64) []byte { + var a [binary.MaxVarintLen64]byte + n := binary.PutUvarint(a[:], x) + return append(b, a[:n]...) +} diff --git a/crypto/multisig/compact_bit_array_test.go b/crypto/multisig/compact_bit_array_test.go new file mode 100644 index 000000000..91a82192f --- /dev/null +++ b/crypto/multisig/compact_bit_array_test.go @@ -0,0 +1,169 @@ +package multisig + +import ( + "encoding/json" + "math/rand" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + cmn "github.com/tendermint/tendermint/libs/common" +) + +func randCompactBitArray(bits int) (*CompactBitArray, []byte) { + numBytes := (bits + 7) / 8 + src := cmn.RandBytes((bits + 7) / 8) + bA := NewCompactBitArray(bits) + + for i := 0; i < numBytes-1; i++ { + for j := uint8(0); j < 8; j++ { + bA.SetIndex(i*8+int(j), src[i]&(uint8(1)<<(8-j)) > 0) + } + } + // Set remaining bits + for i := uint8(0); i < 8-uint8(bA.ExtraBitsStored); i++ { + bA.SetIndex(numBytes*8+int(i), src[numBytes-1]&(uint8(1)<<(8-i)) > 0) + } + return bA, src +} + +func TestNewBitArrayNeverCrashesOnNegatives(t *testing.T) { + bitList := []int{-127, -128, -1 << 31} + for _, bits := range bitList { + _ = NewCompactBitArray(bits) + } +} + +func TestJSONMarshalUnmarshal(t *testing.T) { + + bA1 := NewCompactBitArray(0) + bA2 := NewCompactBitArray(1) + + bA3 := NewCompactBitArray(1) + bA3.SetIndex(0, true) + + bA4 := NewCompactBitArray(5) + bA4.SetIndex(0, true) + bA4.SetIndex(1, true) + + bA5 := NewCompactBitArray(9) + bA5.SetIndex(0, true) + bA5.SetIndex(1, true) + bA5.SetIndex(8, true) + + bA6 := NewCompactBitArray(16) + bA6.SetIndex(0, true) + bA6.SetIndex(1, true) + bA6.SetIndex(8, false) + bA6.SetIndex(15, true) + + testCases := []struct { + bA *CompactBitArray + marshalledBA string + }{ + {nil, `null`}, + {bA1, `null`}, + {bA2, `"_"`}, + {bA3, `"x"`}, + {bA4, `"xx___"`}, + {bA5, `"xx______x"`}, + {bA6, `"xx_____________x"`}, + } + + for _, tc := range testCases { + t.Run(tc.bA.String(), func(t *testing.T) { + bz, err := json.Marshal(tc.bA) + require.NoError(t, err) + + assert.Equal(t, tc.marshalledBA, string(bz)) + + var unmarshalledBA *CompactBitArray + err = json.Unmarshal(bz, &unmarshalledBA) + require.NoError(t, err) + + if tc.bA == nil { + require.Nil(t, unmarshalledBA) + } else { + require.NotNil(t, unmarshalledBA) + assert.EqualValues(t, tc.bA.Elems, unmarshalledBA.Elems) + if assert.EqualValues(t, tc.bA.String(), unmarshalledBA.String()) { + assert.EqualValues(t, tc.bA.Elems, unmarshalledBA.Elems) + } + } + }) + } +} + +func TestCompactMarshalUnmarshal(t *testing.T) { + bA1 := NewCompactBitArray(0) + bA2 := NewCompactBitArray(1) + + bA3 := NewCompactBitArray(1) + bA3.SetIndex(0, true) + + bA4 := NewCompactBitArray(5) + bA4.SetIndex(0, true) + bA4.SetIndex(1, true) + + bA5 := NewCompactBitArray(9) + bA5.SetIndex(0, true) + bA5.SetIndex(1, true) + bA5.SetIndex(8, true) + + bA6 := NewCompactBitArray(16) + bA6.SetIndex(0, true) + bA6.SetIndex(1, true) + bA6.SetIndex(8, false) + bA6.SetIndex(15, true) + + testCases := []struct { + bA *CompactBitArray + marshalledBA []byte + }{ + {nil, []byte("null")}, + {bA1, []byte("null")}, + {bA2, []byte{byte(1), byte(0)}}, + {bA3, []byte{byte(1), byte(128)}}, + {bA4, []byte{byte(5), byte(192)}}, + {bA5, []byte{byte(9), byte(192), byte(128)}}, + {bA6, []byte{byte(16), byte(192), byte(1)}}, + } + + for _, tc := range testCases { + t.Run(tc.bA.String(), func(t *testing.T) { + bz := tc.bA.CompactMarshal() + + assert.Equal(t, tc.marshalledBA, bz) + + unmarshalledBA, err := CompactUnmarshal(bz) + require.NoError(t, err) + if tc.bA == nil { + require.Nil(t, unmarshalledBA) + } else { + require.NotNil(t, unmarshalledBA) + assert.EqualValues(t, tc.bA.Elems, unmarshalledBA.Elems) + if assert.EqualValues(t, tc.bA.String(), unmarshalledBA.String()) { + assert.EqualValues(t, tc.bA.Elems, unmarshalledBA.Elems) + } + } + }) + } +} + +func TestCompactBitArrayGetSetIndex(t *testing.T) { + r := rand.New(rand.NewSource(100)) + numTests := 10 + numBitsPerArr := 100 + for i := 0; i < numTests; i++ { + bits := r.Intn(1000) + bA, _ := randCompactBitArray(bits) + + for j := 0; j < numBitsPerArr; j++ { + copy := bA.Copy() + index := r.Intn(bits) + val := (r.Int63() % 2) == 0 + bA.SetIndex(index, val) + require.Equal(t, val, bA.GetIndex(index), "bA.SetIndex(%d, %v) failed on bit array: %s", index, val, copy) + } + } +} From 21448bcf4f434f74bd1768e1bf3ba4a001db953e Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Thu, 2 Aug 2018 23:18:09 -0700 Subject: [PATCH 02/10] crypto: Add compact bit array for intended usage in the multisig This is in a separate PR for ease of review. --- crypto/multisig/compact_bit_array.go | 218 ++++++++++++++++++++++ crypto/multisig/compact_bit_array_test.go | 169 +++++++++++++++++ 2 files changed, 387 insertions(+) create mode 100644 crypto/multisig/compact_bit_array.go create mode 100644 crypto/multisig/compact_bit_array_test.go diff --git a/crypto/multisig/compact_bit_array.go b/crypto/multisig/compact_bit_array.go new file mode 100644 index 000000000..d14dd8e67 --- /dev/null +++ b/crypto/multisig/compact_bit_array.go @@ -0,0 +1,218 @@ +package multisig + +import ( + "bytes" + "encoding/binary" + "errors" + "fmt" + "regexp" + "strings" +) + +// CompactBitArray is an implementation of a space efficient bit array. +// This is used to ensure that the encoded data takes up a minimal amount of +// space after amino encoding. +// This is not thread safe, and is not intended for concurrent usage. +type CompactBitArray struct { + ExtraBitsStored byte `json:"extra_bits"` // The number of extra bits in elems. + Elems []byte `json:"bits"` +} + +// NewCompactBitArray returns a new compact bit array. +// It returns nil if the number of bits is zero. +func NewCompactBitArray(bits int) *CompactBitArray { + if bits <= 0 { + return nil + } + return &CompactBitArray{ + ExtraBitsStored: byte(bits % 8), + Elems: make([]byte, (bits+7)/8), + } +} + +// Size returns the number of bits in the bitarray +func (bA *CompactBitArray) Size() int { + if bA == nil { + return 0 + } else if bA.ExtraBitsStored == byte(0) { + return len(bA.Elems) * 8 + } + return (len(bA.Elems)-1)*8 + int(bA.ExtraBitsStored) +} + +// GetIndex returns the bit at index i within the bit array. +// The behavior is undefined if i >= bA.Size() +func (bA *CompactBitArray) GetIndex(i int) bool { + if bA == nil { + return false + } + if i >= bA.Size() { + return false + } + return bA.Elems[i>>3]&(uint8(1)< 0 +} + +// SetIndex sets the bit at index i within the bit array. +// The behavior is undefined if i >= bA.Size() +func (bA *CompactBitArray) SetIndex(i int, v bool) bool { + if bA == nil { + return false + } + if i >= bA.Size() { + return false + } + if v { + bA.Elems[i>>3] |= (uint8(1) << uint8(7-(i%8))) + } else { + bA.Elems[i>>3] &= ^(uint8(1) << uint8(7-(i%8))) + } + return true +} + +// Copy returns a copy of the provided bit array. +func (bA *CompactBitArray) Copy() *CompactBitArray { + if bA == nil { + return nil + } + c := make([]byte, len(bA.Elems)) + copy(c, bA.Elems) + return &CompactBitArray{ + ExtraBitsStored: bA.ExtraBitsStored, + Elems: c, + } +} + +// String returns a string representation of CompactBitArray: BA{}, +// where is a sequence of 'x' (1) and '_' (0). +// The includes spaces and newlines to help people. +// For a simple sequence of 'x' and '_' characters with no spaces or newlines, +// see the MarshalJSON() method. +// Example: "BA{_x_}" or "nil-BitArray" for nil. +func (bA *CompactBitArray) String() string { + return bA.StringIndented("") +} + +// StringIndented returns the same thing as String(), but applies the indent +// at every 10th bit, and twice at every 50th bit. +func (bA *CompactBitArray) StringIndented(indent string) string { + if bA == nil { + return "nil-BitArray" + } + lines := []string{} + bits := "" + size := bA.Size() + for i := 0; i < size; i++ { + if bA.GetIndex(i) { + bits += "x" + } else { + bits += "_" + } + if i%100 == 99 { + lines = append(lines, bits) + bits = "" + } + if i%10 == 9 { + bits += indent + } + if i%50 == 49 { + bits += indent + } + } + if len(bits) > 0 { + lines = append(lines, bits) + } + return fmt.Sprintf("BA{%v:%v}", size, strings.Join(lines, indent)) +} + +// MarshalJSON implements json.Marshaler interface by marshaling bit array +// using a custom format: a string of '-' or 'x' where 'x' denotes the 1 bit. +func (bA *CompactBitArray) MarshalJSON() ([]byte, error) { + if bA == nil { + return []byte("null"), nil + } + + bits := `"` + size := bA.Size() + for i := 0; i < size; i++ { + if bA.GetIndex(i) { + bits += `x` + } else { + bits += `_` + } + } + bits += `"` + return []byte(bits), nil +} + +var bitArrayJSONRegexp = regexp.MustCompile(`\A"([_x]*)"\z`) + +// UnmarshalJSON implements json.Unmarshaler interface by unmarshaling a custom +// JSON description. +func (bA *CompactBitArray) UnmarshalJSON(bz []byte) error { + b := string(bz) + if b == "null" { + // This is required e.g. for encoding/json when decoding + // into a pointer with pre-allocated BitArray. + bA.ExtraBitsStored = 0 + bA.Elems = nil + return nil + } + + // Validate 'b'. + match := bitArrayJSONRegexp.FindStringSubmatch(b) + if match == nil { + return fmt.Errorf("BitArray in JSON should be a string of format %q but got %s", bitArrayJSONRegexp.String(), b) + } + bits := match[1] + + // Construct new CompactBitArray and copy over. + numBits := len(bits) + bA2 := NewCompactBitArray(numBits) + for i := 0; i < numBits; i++ { + if bits[i] == 'x' { + bA2.SetIndex(i, true) + } + } + *bA = *bA2 + return nil +} + +// CompactMarshal is a space efficient encoding for CompactBitArray. +// It is not amino compatible. +func (bA *CompactBitArray) CompactMarshal() []byte { + size := bA.Size() + if size <= 0 { + return []byte("null") + } + bz := make([]byte, 0, size/8) + // length prefix number of bits, not number of bytes. This difference + // takes 3-4 bits in encoding, as opposed to instead encoding the number of + // bytes (saving 3-4 bits) and including the offset as a full byte. + bz = appendUvarint(bz, uint64(size)) + bz = append(bz, bA.Elems...) + return bz +} + +// CompactUnmarshal is a space efficient decoding for CompactBitArray. +// It is not amino compatible. +func CompactUnmarshal(bz []byte) (*CompactBitArray, error) { + if len(bz) < 2 { + return nil, errors.New("compact bit array: invalid compact unmarshal size") + } else if bytes.Equal(bz, []byte("null")) { + return NewCompactBitArray(0), nil + } + size, n := binary.Uvarint(bz) + bz = bz[n:] + if len(bz) != int(size+7)/8 { + return nil, errors.New("compact bit array: invalid compact unmarshal size") + } + + bA := &CompactBitArray{byte(int(size % 8)), bz} + return bA, nil +} + +func appendUvarint(b []byte, x uint64) []byte { + var a [binary.MaxVarintLen64]byte + n := binary.PutUvarint(a[:], x) + return append(b, a[:n]...) +} diff --git a/crypto/multisig/compact_bit_array_test.go b/crypto/multisig/compact_bit_array_test.go new file mode 100644 index 000000000..91a82192f --- /dev/null +++ b/crypto/multisig/compact_bit_array_test.go @@ -0,0 +1,169 @@ +package multisig + +import ( + "encoding/json" + "math/rand" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + cmn "github.com/tendermint/tendermint/libs/common" +) + +func randCompactBitArray(bits int) (*CompactBitArray, []byte) { + numBytes := (bits + 7) / 8 + src := cmn.RandBytes((bits + 7) / 8) + bA := NewCompactBitArray(bits) + + for i := 0; i < numBytes-1; i++ { + for j := uint8(0); j < 8; j++ { + bA.SetIndex(i*8+int(j), src[i]&(uint8(1)<<(8-j)) > 0) + } + } + // Set remaining bits + for i := uint8(0); i < 8-uint8(bA.ExtraBitsStored); i++ { + bA.SetIndex(numBytes*8+int(i), src[numBytes-1]&(uint8(1)<<(8-i)) > 0) + } + return bA, src +} + +func TestNewBitArrayNeverCrashesOnNegatives(t *testing.T) { + bitList := []int{-127, -128, -1 << 31} + for _, bits := range bitList { + _ = NewCompactBitArray(bits) + } +} + +func TestJSONMarshalUnmarshal(t *testing.T) { + + bA1 := NewCompactBitArray(0) + bA2 := NewCompactBitArray(1) + + bA3 := NewCompactBitArray(1) + bA3.SetIndex(0, true) + + bA4 := NewCompactBitArray(5) + bA4.SetIndex(0, true) + bA4.SetIndex(1, true) + + bA5 := NewCompactBitArray(9) + bA5.SetIndex(0, true) + bA5.SetIndex(1, true) + bA5.SetIndex(8, true) + + bA6 := NewCompactBitArray(16) + bA6.SetIndex(0, true) + bA6.SetIndex(1, true) + bA6.SetIndex(8, false) + bA6.SetIndex(15, true) + + testCases := []struct { + bA *CompactBitArray + marshalledBA string + }{ + {nil, `null`}, + {bA1, `null`}, + {bA2, `"_"`}, + {bA3, `"x"`}, + {bA4, `"xx___"`}, + {bA5, `"xx______x"`}, + {bA6, `"xx_____________x"`}, + } + + for _, tc := range testCases { + t.Run(tc.bA.String(), func(t *testing.T) { + bz, err := json.Marshal(tc.bA) + require.NoError(t, err) + + assert.Equal(t, tc.marshalledBA, string(bz)) + + var unmarshalledBA *CompactBitArray + err = json.Unmarshal(bz, &unmarshalledBA) + require.NoError(t, err) + + if tc.bA == nil { + require.Nil(t, unmarshalledBA) + } else { + require.NotNil(t, unmarshalledBA) + assert.EqualValues(t, tc.bA.Elems, unmarshalledBA.Elems) + if assert.EqualValues(t, tc.bA.String(), unmarshalledBA.String()) { + assert.EqualValues(t, tc.bA.Elems, unmarshalledBA.Elems) + } + } + }) + } +} + +func TestCompactMarshalUnmarshal(t *testing.T) { + bA1 := NewCompactBitArray(0) + bA2 := NewCompactBitArray(1) + + bA3 := NewCompactBitArray(1) + bA3.SetIndex(0, true) + + bA4 := NewCompactBitArray(5) + bA4.SetIndex(0, true) + bA4.SetIndex(1, true) + + bA5 := NewCompactBitArray(9) + bA5.SetIndex(0, true) + bA5.SetIndex(1, true) + bA5.SetIndex(8, true) + + bA6 := NewCompactBitArray(16) + bA6.SetIndex(0, true) + bA6.SetIndex(1, true) + bA6.SetIndex(8, false) + bA6.SetIndex(15, true) + + testCases := []struct { + bA *CompactBitArray + marshalledBA []byte + }{ + {nil, []byte("null")}, + {bA1, []byte("null")}, + {bA2, []byte{byte(1), byte(0)}}, + {bA3, []byte{byte(1), byte(128)}}, + {bA4, []byte{byte(5), byte(192)}}, + {bA5, []byte{byte(9), byte(192), byte(128)}}, + {bA6, []byte{byte(16), byte(192), byte(1)}}, + } + + for _, tc := range testCases { + t.Run(tc.bA.String(), func(t *testing.T) { + bz := tc.bA.CompactMarshal() + + assert.Equal(t, tc.marshalledBA, bz) + + unmarshalledBA, err := CompactUnmarshal(bz) + require.NoError(t, err) + if tc.bA == nil { + require.Nil(t, unmarshalledBA) + } else { + require.NotNil(t, unmarshalledBA) + assert.EqualValues(t, tc.bA.Elems, unmarshalledBA.Elems) + if assert.EqualValues(t, tc.bA.String(), unmarshalledBA.String()) { + assert.EqualValues(t, tc.bA.Elems, unmarshalledBA.Elems) + } + } + }) + } +} + +func TestCompactBitArrayGetSetIndex(t *testing.T) { + r := rand.New(rand.NewSource(100)) + numTests := 10 + numBitsPerArr := 100 + for i := 0; i < numTests; i++ { + bits := r.Intn(1000) + bA, _ := randCompactBitArray(bits) + + for j := 0; j < numBitsPerArr; j++ { + copy := bA.Copy() + index := r.Intn(bits) + val := (r.Int63() % 2) == 0 + bA.SetIndex(index, val) + require.Equal(t, val, bA.GetIndex(index), "bA.SetIndex(%d, %v) failed on bit array: %s", index, val, copy) + } + } +} From e7dd76c28d9eb3ff9b30f0dd1303e25bd16bca9b Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Mon, 6 Aug 2018 21:17:38 -0500 Subject: [PATCH 03/10] crypto: Threshold multisig implementation --- crypto/multisig/compact_bit_array.go | 15 +++++ crypto/multisig/compact_bit_array_test.go | 26 ++++++++ crypto/multisig/multisignature.go | 60 +++++++++++++++++ crypto/multisig/threshold_multisig.go | 78 ++++++++++++++++++++++ crypto/multisig/threshold_multisig_test.go | 73 ++++++++++++++++++++ crypto/multisig/wire.go | 26 ++++++++ 6 files changed, 278 insertions(+) create mode 100644 crypto/multisig/multisignature.go create mode 100644 crypto/multisig/threshold_multisig.go create mode 100644 crypto/multisig/threshold_multisig_test.go create mode 100644 crypto/multisig/wire.go diff --git a/crypto/multisig/compact_bit_array.go b/crypto/multisig/compact_bit_array.go index d14dd8e67..f7f508ce4 100644 --- a/crypto/multisig/compact_bit_array.go +++ b/crypto/multisig/compact_bit_array.go @@ -69,6 +69,21 @@ func (bA *CompactBitArray) SetIndex(i int, v bool) bool { return true } +// trueIndex returns the location of the given index, among the +// values in the bit array that are set to true. +// e.g. if bA = _XX_X_X, trueIndex(4) = 2, since +// the value at index 4 of the bit array is the third +// value that is true. (And it is 0-indexed) +func (bA *CompactBitArray) trueIndex(index int) int { + numTrueValues := 0 + for i := 0; i < index; i++ { + if bA.GetIndex(i) { + numTrueValues++ + } + } + return numTrueValues +} + // Copy returns a copy of the provided bit array. func (bA *CompactBitArray) Copy() *CompactBitArray { if bA == nil { diff --git a/crypto/multisig/compact_bit_array_test.go b/crypto/multisig/compact_bit_array_test.go index 91a82192f..dd3bed76b 100644 --- a/crypto/multisig/compact_bit_array_test.go +++ b/crypto/multisig/compact_bit_array_test.go @@ -150,6 +150,32 @@ func TestCompactMarshalUnmarshal(t *testing.T) { } } +func TestCompactBitArrayTrueIndex(t *testing.T) { + testCases := []struct { + marshalledBA string + bAIndex []int + trueValueIndex []int + }{ + {`"_____"`, []int{0, 1, 2, 3, 4}, []int{0, 0, 0, 0, 0}}, + {`"x"`, []int{0}, []int{0}}, + {`"_x"`, []int{1}, []int{0}}, + {`"x___xxxx"`, []int{0, 4, 5, 6, 7}, []int{0, 1, 2, 3, 4}}, + {`"__x_xx_x__x_x___"`, []int{2, 4, 5, 7, 10, 12}, []int{0, 1, 2, 3, 4, 5}}, + {`"______________xx"`, []int{14, 15}, []int{0, 1}}, + } + for tcIndex, tc := range testCases { + t.Run(tc.marshalledBA, func(t *testing.T) { + var bA *CompactBitArray + err := json.Unmarshal([]byte(tc.marshalledBA), &bA) + require.NoError(t, err) + + for i := 0; i < len(tc.bAIndex); i++ { + require.Equal(t, tc.trueValueIndex[i], bA.trueIndex(tc.bAIndex[i]), "tc %d, i %d", tcIndex, i) + } + }) + } +} + func TestCompactBitArrayGetSetIndex(t *testing.T) { r := rand.New(rand.NewSource(100)) numTests := 10 diff --git a/crypto/multisig/multisignature.go b/crypto/multisig/multisignature.go new file mode 100644 index 000000000..60300ef56 --- /dev/null +++ b/crypto/multisig/multisignature.go @@ -0,0 +1,60 @@ +package multisig + +import "github.com/tendermint/tendermint/crypto" + +// 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 + Sigs [][]byte +} + +// NewMultisig returns a new Multisignature of size n. +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)} +} + +// GetIndex returns the index of pk in keys. Returns -1 if not found +func GetIndex(pk crypto.PubKey, keys []crypto.PubKey) int { + for i := 0; i < len(keys); i++ { + if pk.Equals(keys[i]) { + return i + } + } + return -1 +} + +// AddSignature adds a signature to the multisig, at the corresponding index. +func (mSig *Multisignature) AddSignature(sig []byte, index int) { + i := mSig.BitArray.trueIndex(index) + // Signature already exists, just replace the value there + if mSig.BitArray.GetIndex(index) { + mSig.Sigs[i] = sig + return + } + mSig.BitArray.SetIndex(index, true) + // Optimization if the index is the greatest index + if i > len(mSig.Sigs) { + mSig.Sigs = append(mSig.Sigs, sig) + return + } + // Expand slice by one with a dummy element, move all elements after i + // over by one, then place the new signature in that gap. + mSig.Sigs = append(mSig.Sigs, make([]byte, 0)) + copy(mSig.Sigs[i+1:], mSig.Sigs[i:]) + mSig.Sigs[i] = sig +} + +// 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) { + index := GetIndex(pubkey, keys) + mSig.AddSignature(sig, index) +} + +// Marshal the multisignature with amino +func (mSig *Multisignature) Marshal() []byte { + return cdc.MustMarshalBinary(mSig) +} diff --git a/crypto/multisig/threshold_multisig.go b/crypto/multisig/threshold_multisig.go new file mode 100644 index 000000000..79438f89c --- /dev/null +++ b/crypto/multisig/threshold_multisig.go @@ -0,0 +1,78 @@ +package multisig + +import ( + "github.com/tendermint/tendermint/crypto" + "github.com/tendermint/tendermint/crypto/tmhash" +) + +// ThresholdMultiSignaturePubKey implements a K of N threshold multisig +type ThresholdMultiSignaturePubKey struct { + K uint `json:"threshold"` + Pubkeys []crypto.PubKey `json:"pubkeys"` +} + +var _ crypto.PubKey = &ThresholdMultiSignaturePubKey{} + +// NewThresholdMultiSignaturePubKey returns a new ThresholdMultiSignaturePubKey. +func NewThresholdMultiSignaturePubKey(k int, pubkeys []crypto.PubKey) crypto.PubKey { + if len(pubkeys) < k { + panic("threshold k of n multisignature: len(pubkeys) < k") + } + return &ThresholdMultiSignaturePubKey{uint(k), pubkeys} +} + +// VerifyBytes 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. +func (pk *ThresholdMultiSignaturePubKey) VerifyBytes(msg []byte, marshalledSig []byte) bool { + var sig *Multisignature + err := cdc.UnmarshalBinary(marshalledSig, &sig) + if err != nil { + return false + } + size := sig.BitArray.Size() + if len(sig.Sigs) < int(pk.K) || len(pk.Pubkeys) != size { + 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].VerifyBytes(msg, sig.Sigs[sigIndex]) { + return false + } + sigIndex++ + } + } + return true +} + +// Bytes returns the amino encoded version of the ThresholdMultiSignaturePubKey +func (pk *ThresholdMultiSignaturePubKey) Bytes() []byte { + return cdc.MustMarshalBinary(pk) +} + +// Address returns tmhash(ThresholdMultiSignaturePubKey.Bytes()) +func (pk *ThresholdMultiSignaturePubKey) 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 { + if otherKey, ok := other.(*ThresholdMultiSignaturePubKey); ok { + if pk.K != otherKey.K { + return false + } + for i := uint(0); i < pk.K; i++ { + if !pk.Pubkeys[i].Equals(otherKey.Pubkeys[i]) { + return false + } + } + return true + } + return false +} diff --git a/crypto/multisig/threshold_multisig_test.go b/crypto/multisig/threshold_multisig_test.go new file mode 100644 index 000000000..31da13192 --- /dev/null +++ b/crypto/multisig/threshold_multisig_test.go @@ -0,0 +1,73 @@ +package multisig + +import ( + "math/rand" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/tendermint/tendermint/crypto" + "github.com/tendermint/tendermint/crypto/ed25519" + "github.com/tendermint/tendermint/crypto/secp256k1" +) + +func TestThresholdMultisig(t *testing.T) { + msg := []byte{1, 2, 3, 4} + pubkeys, sigs := generatePubKeysAndSignatures(5, msg) + multisigKey := NewThresholdMultiSignaturePubKey(2, pubkeys) + multisignature := NewMultisig(5) + require.False(t, multisigKey.VerifyBytes(msg, multisignature.Marshal())) + multisignature.AddSignatureFromPubkey(sigs[0], pubkeys[0], pubkeys) + require.False(t, multisigKey.VerifyBytes(msg, multisignature.Marshal())) + // Make sure adding the same signature twice doesn't make the signature pass + multisignature.AddSignatureFromPubkey(sigs[0], pubkeys[0], pubkeys) + require.False(t, multisigKey.VerifyBytes(msg, multisignature.Marshal())) + + // Adding two signatures should make it pass, as k = 2 + multisignature.AddSignatureFromPubkey(sigs[3], pubkeys[3], pubkeys) + require.True(t, multisigKey.VerifyBytes(msg, multisignature.Marshal())) + + // Adding a third invalid signature should make verification fail. + multisignature.AddSignatureFromPubkey(sigs[0], pubkeys[4], pubkeys) + require.False(t, multisigKey.VerifyBytes(msg, multisignature.Marshal())) + + // try adding the invalid signature one signature before + // first reset the multisig + multisignature.BitArray.SetIndex(4, false) + multisignature.Sigs = multisignature.Sigs[:2] + multisignature.AddSignatureFromPubkey(sigs[0], pubkeys[2], pubkeys) + require.False(t, multisigKey.VerifyBytes(msg, multisignature.Marshal())) +} + +func TestMultiSigPubkeyEquality(t *testing.T) { + msg := []byte{1, 2, 3, 4} + pubkeys, _ := generatePubKeysAndSignatures(5, msg) + multisigKey := NewThresholdMultiSignaturePubKey(2, pubkeys) + var unmarshalledMultisig *ThresholdMultiSignaturePubKey + cdc.MustUnmarshalBinary(multisigKey.Bytes(), &unmarshalledMultisig) + require.Equal(t, multisigKey, unmarshalledMultisig) + + // Ensure that reordering pubkeys is treated as a different pubkey + pubkeysCpy := make([]crypto.PubKey, 5) + copy(pubkeysCpy, pubkeys) + pubkeysCpy[4] = pubkeys[3] + pubkeysCpy[3] = pubkeys[4] + multisigKey2 := NewThresholdMultiSignaturePubKey(2, pubkeysCpy) + require.NotEqual(t, multisigKey, multisigKey2) +} + +func generatePubKeysAndSignatures(n int, msg []byte) (pubkeys []crypto.PubKey, signatures [][]byte) { + pubkeys = make([]crypto.PubKey, n) + signatures = make([][]byte, n) + for i := 0; i < n; i++ { + var privkey crypto.PrivKey + if rand.Int63()%2 == 0 { + privkey = ed25519.GenPrivKey() + } else { + privkey = secp256k1.GenPrivKey() + } + pubkeys[i] = privkey.PubKey() + signatures[i], _ = privkey.Sign(msg) + } + return +} diff --git a/crypto/multisig/wire.go b/crypto/multisig/wire.go new file mode 100644 index 000000000..e0cb2a591 --- /dev/null +++ b/crypto/multisig/wire.go @@ -0,0 +1,26 @@ +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/secp256k1" +) + +// TODO: Figure out API for others to either add their own pubkey types, or +// to make verify / marshal accept a cdc. +const ( + ThresholdPubkeyAminoRoute = "tendermint/ThresholdMultisigPubkey" +) + +var cdc = amino.NewCodec() + +func init() { + cdc.RegisterInterface((*crypto.PubKey)(nil), nil) + cdc.RegisterConcrete(ThresholdMultiSignaturePubKey{}, + ThresholdPubkeyAminoRoute, nil) + cdc.RegisterConcrete(ed25519.PubKeyEd25519{}, + ed25519.Ed25519PubKeyAminoRoute, nil) + cdc.RegisterConcrete(secp256k1.PubKeySecp256k1{}, + secp256k1.Secp256k1PubKeyAminoRoute, nil) +} From 67b6d51ff4333c209aee515c3e0cf85318d1d043 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Tue, 7 Aug 2018 10:42:47 -0500 Subject: [PATCH 04/10] (squash this) address PR comments + fix bug in equality check --- crypto/multisig/compact_bit_array.go | 6 ++--- crypto/multisig/compact_bit_array_test.go | 4 ++-- crypto/multisig/multisignature.go | 26 ++++++++++++++-------- crypto/multisig/threshold_multisig.go | 6 ++--- crypto/multisig/threshold_multisig_test.go | 4 ++-- 5 files changed, 27 insertions(+), 19 deletions(-) diff --git a/crypto/multisig/compact_bit_array.go b/crypto/multisig/compact_bit_array.go index f7f508ce4..2d9fe72ab 100644 --- a/crypto/multisig/compact_bit_array.go +++ b/crypto/multisig/compact_bit_array.go @@ -69,12 +69,12 @@ func (bA *CompactBitArray) SetIndex(i int, v bool) bool { return true } -// trueIndex returns the location of the given index, among the +// NumOfTrueBitsBefore returns the location of the given index, among the // values in the bit array that are set to true. -// e.g. if bA = _XX_X_X, trueIndex(4) = 2, since +// e.g. if bA = _XX_X_X, NumOfTrueBitsBefore(4) = 2, since // the value at index 4 of the bit array is the third // value that is true. (And it is 0-indexed) -func (bA *CompactBitArray) trueIndex(index int) int { +func (bA *CompactBitArray) NumOfTrueBitsBefore(index int) int { numTrueValues := 0 for i := 0; i < index; i++ { if bA.GetIndex(i) { diff --git a/crypto/multisig/compact_bit_array_test.go b/crypto/multisig/compact_bit_array_test.go index dd3bed76b..3382c8b93 100644 --- a/crypto/multisig/compact_bit_array_test.go +++ b/crypto/multisig/compact_bit_array_test.go @@ -150,7 +150,7 @@ func TestCompactMarshalUnmarshal(t *testing.T) { } } -func TestCompactBitArrayTrueIndex(t *testing.T) { +func TestCompactBitArrayNumOfTrueBitsBefore(t *testing.T) { testCases := []struct { marshalledBA string bAIndex []int @@ -170,7 +170,7 @@ func TestCompactBitArrayTrueIndex(t *testing.T) { require.NoError(t, err) for i := 0; i < len(tc.bAIndex); i++ { - require.Equal(t, tc.trueValueIndex[i], bA.trueIndex(tc.bAIndex[i]), "tc %d, i %d", tcIndex, i) + require.Equal(t, tc.trueValueIndex[i], bA.NumOfTrueBitsBefore(tc.bAIndex[i]), "tc %d, i %d", tcIndex, i) } }) } diff --git a/crypto/multisig/multisignature.go b/crypto/multisig/multisignature.go index 60300ef56..bd2398b83 100644 --- a/crypto/multisig/multisignature.go +++ b/crypto/multisig/multisignature.go @@ -1,6 +1,10 @@ package multisig -import "github.com/tendermint/tendermint/crypto" +import ( + "errors" + + "github.com/tendermint/tendermint/crypto" +) // Multisignature is used to represent the signature object used in the multisigs. // Sigs is a list of signatures, sorted by corresponding index. @@ -17,7 +21,7 @@ func NewMultisig(n int) *Multisignature { } // GetIndex returns the index of pk in keys. Returns -1 if not found -func GetIndex(pk crypto.PubKey, keys []crypto.PubKey) int { +func getIndex(pk crypto.PubKey, keys []crypto.PubKey) int { for i := 0; i < len(keys); i++ { if pk.Equals(keys[i]) { return i @@ -28,30 +32,34 @@ func GetIndex(pk crypto.PubKey, keys []crypto.PubKey) int { // AddSignature adds a signature to the multisig, at the corresponding index. func (mSig *Multisignature) AddSignature(sig []byte, index int) { - i := mSig.BitArray.trueIndex(index) + newSigIndex := mSig.BitArray.NumOfTrueBitsBefore(index) // Signature already exists, just replace the value there if mSig.BitArray.GetIndex(index) { - mSig.Sigs[i] = sig + mSig.Sigs[newSigIndex] = sig return } mSig.BitArray.SetIndex(index, true) // Optimization if the index is the greatest index - if i > len(mSig.Sigs) { + if newSigIndex == len(mSig.Sigs) { mSig.Sigs = append(mSig.Sigs, sig) return } // Expand slice by one with a dummy element, move all elements after i // over by one, then place the new signature in that gap. mSig.Sigs = append(mSig.Sigs, make([]byte, 0)) - copy(mSig.Sigs[i+1:], mSig.Sigs[i:]) - mSig.Sigs[i] = sig + copy(mSig.Sigs[newSigIndex+1:], mSig.Sigs[newSigIndex:]) + mSig.Sigs[newSigIndex] = sig } // 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) { - index := GetIndex(pubkey, keys) +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") + } mSig.AddSignature(sig, index) + return nil } // Marshal the multisignature with amino diff --git a/crypto/multisig/threshold_multisig.go b/crypto/multisig/threshold_multisig.go index 79438f89c..4ea69050e 100644 --- a/crypto/multisig/threshold_multisig.go +++ b/crypto/multisig/threshold_multisig.go @@ -5,7 +5,7 @@ import ( "github.com/tendermint/tendermint/crypto/tmhash" ) -// ThresholdMultiSignaturePubKey implements a K of N threshold multisig +// ThresholdMultiSignaturePubKey implements a K of N threshold multisig. type ThresholdMultiSignaturePubKey struct { K uint `json:"threshold"` Pubkeys []crypto.PubKey `json:"pubkeys"` @@ -64,10 +64,10 @@ func (pk *ThresholdMultiSignaturePubKey) Address() crypto.Address { // all constituent keys are the same, and in the same order. func (pk *ThresholdMultiSignaturePubKey) Equals(other crypto.PubKey) bool { if otherKey, ok := other.(*ThresholdMultiSignaturePubKey); ok { - if pk.K != otherKey.K { + if pk.K != otherKey.K || len(pk.Pubkeys) != len(otherKey.Pubkeys) { return false } - for i := uint(0); i < pk.K; 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_multisig_test.go index 31da13192..c3c4263f0 100644 --- a/crypto/multisig/threshold_multisig_test.go +++ b/crypto/multisig/threshold_multisig_test.go @@ -45,7 +45,7 @@ func TestMultiSigPubkeyEquality(t *testing.T) { multisigKey := NewThresholdMultiSignaturePubKey(2, pubkeys) var unmarshalledMultisig *ThresholdMultiSignaturePubKey cdc.MustUnmarshalBinary(multisigKey.Bytes(), &unmarshalledMultisig) - require.Equal(t, multisigKey, unmarshalledMultisig) + require.True(t, multisigKey.Equals(unmarshalledMultisig)) // Ensure that reordering pubkeys is treated as a different pubkey pubkeysCpy := make([]crypto.PubKey, 5) @@ -53,7 +53,7 @@ func TestMultiSigPubkeyEquality(t *testing.T) { pubkeysCpy[4] = pubkeys[3] pubkeysCpy[3] = pubkeys[4] multisigKey2 := NewThresholdMultiSignaturePubKey(2, pubkeysCpy) - require.NotEqual(t, multisigKey, multisigKey2) + require.False(t, multisigKey.Equals(multisigKey2)) } func generatePubKeysAndSignatures(n int, msg []byte) (pubkeys []crypto.PubKey, signatures [][]byte) { From 4e7bf10b599ab15e951daa0cbe77df68a8c56659 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Tue, 7 Aug 2018 11:37:42 -0500 Subject: [PATCH 05/10] (squash this) squashed bug with multiple signatures at same index. --- crypto/multisig/threshold_multisig.go | 2 +- crypto/multisig/threshold_multisig_test.go | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/crypto/multisig/threshold_multisig.go b/crypto/multisig/threshold_multisig.go index 4ea69050e..adc5fd129 100644 --- a/crypto/multisig/threshold_multisig.go +++ b/crypto/multisig/threshold_multisig.go @@ -34,7 +34,7 @@ func (pk *ThresholdMultiSignaturePubKey) VerifyBytes(msg []byte, marshalledSig [ return false } size := sig.BitArray.Size() - if len(sig.Sigs) < int(pk.K) || len(pk.Pubkeys) != size { + if len(sig.Sigs) < int(pk.K) || len(pk.Pubkeys) != size || sig.BitArray.NumOfTrueBitsBefore(size) < int(pk.K) { return false } // index in the list of signatures which we are concerned with. diff --git a/crypto/multisig/threshold_multisig_test.go b/crypto/multisig/threshold_multisig_test.go index c3c4263f0..cbd447a2d 100644 --- a/crypto/multisig/threshold_multisig_test.go +++ b/crypto/multisig/threshold_multisig_test.go @@ -19,9 +19,9 @@ func TestThresholdMultisig(t *testing.T) { require.False(t, multisigKey.VerifyBytes(msg, multisignature.Marshal())) multisignature.AddSignatureFromPubkey(sigs[0], pubkeys[0], pubkeys) require.False(t, multisigKey.VerifyBytes(msg, multisignature.Marshal())) - // Make sure adding the same signature twice doesn't make the signature pass + // Make sure adding the same signature twice doesn't increase number of signatures multisignature.AddSignatureFromPubkey(sigs[0], pubkeys[0], pubkeys) - require.False(t, multisigKey.VerifyBytes(msg, multisignature.Marshal())) + require.Equal(t, 1, len(multisignature.Sigs)) // Adding two signatures should make it pass, as k = 2 multisignature.AddSignatureFromPubkey(sigs[3], pubkeys[3], pubkeys) @@ -39,6 +39,18 @@ func TestThresholdMultisig(t *testing.T) { require.False(t, multisigKey.VerifyBytes(msg, multisignature.Marshal())) } +func TestThresholdMultisigDuplicateSignatures(t *testing.T) { + msg := []byte{1, 2, 3, 4, 5} + pubkeys, sigs := generatePubKeysAndSignatures(5, msg) + multisigKey := NewThresholdMultiSignaturePubKey(2, pubkeys) + multisignature := NewMultisig(5) + require.False(t, multisigKey.VerifyBytes(msg, multisignature.Marshal())) + 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())) +} + func TestMultiSigPubkeyEquality(t *testing.T) { msg := []byte{1, 2, 3, 4} pubkeys, _ := generatePubKeysAndSignatures(5, msg) From 00db469fc05a183ab0af70dcfa128f26d9acd0d7 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Fri, 10 Aug 2018 13:14:43 -0500 Subject: [PATCH 06/10] (squash this) begin addressing PR comments --- crypto/multisig/multisignature.go | 3 +- crypto/multisig/threshold_multisig.go | 36 ++++++---- crypto/multisig/threshold_multisig_test.go | 77 +++++++++++++++------- 3 files changed, 79 insertions(+), 37 deletions(-) diff --git a/crypto/multisig/multisignature.go b/crypto/multisig/multisignature.go index bd2398b83..374b5e1b4 100644 --- a/crypto/multisig/multisignature.go +++ b/crypto/multisig/multisignature.go @@ -31,6 +31,7 @@ func getIndex(pk crypto.PubKey, keys []crypto.PubKey) int { } // AddSignature adds a signature to the multisig, at the corresponding index. +// If the signature already exists, replace it. func (mSig *Multisignature) AddSignature(sig []byte, index int) { newSigIndex := mSig.BitArray.NumOfTrueBitsBefore(index) // Signature already exists, just replace the value there @@ -64,5 +65,5 @@ func (mSig *Multisignature) AddSignatureFromPubkey(sig []byte, pubkey crypto.Pub // Marshal the multisignature with amino func (mSig *Multisignature) Marshal() []byte { - return cdc.MustMarshalBinary(mSig) + return cdc.MustMarshalBinaryBare(mSig) } diff --git a/crypto/multisig/threshold_multisig.go b/crypto/multisig/threshold_multisig.go index adc5fd129..934f7d798 100644 --- a/crypto/multisig/threshold_multisig.go +++ b/crypto/multisig/threshold_multisig.go @@ -14,7 +14,11 @@ type ThresholdMultiSignaturePubKey struct { var _ crypto.PubKey = &ThresholdMultiSignaturePubKey{} // NewThresholdMultiSignaturePubKey returns a new ThresholdMultiSignaturePubKey. +// Panics if len(pubkeys) < k or 0 >= k. func NewThresholdMultiSignaturePubKey(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") } @@ -29,12 +33,21 @@ func NewThresholdMultiSignaturePubKey(k int, pubkeys []crypto.PubKey) crypto.Pub // a concern. func (pk *ThresholdMultiSignaturePubKey) VerifyBytes(msg []byte, marshalledSig []byte) bool { var sig *Multisignature - err := cdc.UnmarshalBinary(marshalledSig, &sig) + err := cdc.UnmarshalBinaryBare(marshalledSig, &sig) if err != nil { return false } size := sig.BitArray.Size() - if len(sig.Sigs) < int(pk.K) || len(pk.Pubkeys) != size || sig.BitArray.NumOfTrueBitsBefore(size) < int(pk.K) { + // 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.NumOfTrueBitsBefore(size) < int(pk.K) { return false } // index in the list of signatures which we are concerned with. @@ -63,16 +76,17 @@ func (pk *ThresholdMultiSignaturePubKey) Address() crypto.Address { // 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 { - if otherKey, ok := other.(*ThresholdMultiSignaturePubKey); ok { - if pk.K != otherKey.K || len(pk.Pubkeys) != len(otherKey.Pubkeys) { + otherKey, sameType := other.(*ThresholdMultiSignaturePubKey) + if !sameType { + return false + } + 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]) { return false } - for i := 0; i < len(pk.Pubkeys); i++ { - if !pk.Pubkeys[i].Equals(otherKey.Pubkeys[i]) { - return false - } - } - return true } - return false + return true } diff --git a/crypto/multisig/threshold_multisig_test.go b/crypto/multisig/threshold_multisig_test.go index cbd447a2d..6036554a3 100644 --- a/crypto/multisig/threshold_multisig_test.go +++ b/crypto/multisig/threshold_multisig_test.go @@ -11,34 +11,60 @@ import ( "github.com/tendermint/tendermint/crypto/secp256k1" ) -func TestThresholdMultisig(t *testing.T) { - msg := []byte{1, 2, 3, 4} - pubkeys, sigs := generatePubKeysAndSignatures(5, msg) - multisigKey := NewThresholdMultiSignaturePubKey(2, pubkeys) - multisignature := NewMultisig(5) - require.False(t, multisigKey.VerifyBytes(msg, multisignature.Marshal())) - multisignature.AddSignatureFromPubkey(sigs[0], pubkeys[0], pubkeys) - require.False(t, multisigKey.VerifyBytes(msg, multisignature.Marshal())) - // Make sure adding the same signature twice doesn't increase number of signatures - multisignature.AddSignatureFromPubkey(sigs[0], pubkeys[0], pubkeys) - require.Equal(t, 1, len(multisignature.Sigs)) - - // Adding two signatures should make it pass, as k = 2 - multisignature.AddSignatureFromPubkey(sigs[3], pubkeys[3], pubkeys) - require.True(t, multisigKey.VerifyBytes(msg, multisignature.Marshal())) - - // Adding a third invalid signature should make verification fail. - multisignature.AddSignatureFromPubkey(sigs[0], pubkeys[4], pubkeys) - require.False(t, multisigKey.VerifyBytes(msg, multisignature.Marshal())) +// 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 [][]byte + 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 := NewThresholdMultiSignaturePubKey(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) + 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) + 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) + 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) + 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) - // try adding the invalid signature one signature before - // first reset the multisig - multisignature.BitArray.SetIndex(4, false) - multisignature.Sigs = multisignature.Sigs[:2] - multisignature.AddSignatureFromPubkey(sigs[0], pubkeys[2], pubkeys) - require.False(t, multisigKey.VerifyBytes(msg, multisignature.Marshal())) + 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) + } + } } +// 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) @@ -51,6 +77,7 @@ func TestThresholdMultisigDuplicateSignatures(t *testing.T) { require.False(t, multisigKey.VerifyBytes(msg, multisignature.Marshal())) } +// TODO: Fully replace this test with table driven tests func TestMultiSigPubkeyEquality(t *testing.T) { msg := []byte{1, 2, 3, 4} pubkeys, _ := generatePubKeysAndSignatures(5, msg) From 4cf1dbd67626d45691057bcbe7e8b018b8925ceb Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Fri, 10 Aug 2018 13:20:59 -0500 Subject: [PATCH 07/10] (squash this) fix amino route --- crypto/multisig/wire.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/multisig/wire.go b/crypto/multisig/wire.go index e0cb2a591..a6cda34a9 100644 --- a/crypto/multisig/wire.go +++ b/crypto/multisig/wire.go @@ -10,7 +10,7 @@ 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/ThresholdMultisigPubkey" + ThresholdPubkeyAminoRoute = "tendermint/PubkeyThresholdMultisig" ) var cdc = amino.NewCodec() From 8d28344e84714ccac00f5876dae344b8d76e458b Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Fri, 10 Aug 2018 13:42:23 -0500 Subject: [PATCH 08/10] (Squash this) switch to bare --- crypto/multisig/threshold_multisig.go | 2 +- crypto/multisig/threshold_multisig_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crypto/multisig/threshold_multisig.go b/crypto/multisig/threshold_multisig.go index 934f7d798..01462d6fb 100644 --- a/crypto/multisig/threshold_multisig.go +++ b/crypto/multisig/threshold_multisig.go @@ -65,7 +65,7 @@ func (pk *ThresholdMultiSignaturePubKey) VerifyBytes(msg []byte, marshalledSig [ // Bytes returns the amino encoded version of the ThresholdMultiSignaturePubKey func (pk *ThresholdMultiSignaturePubKey) Bytes() []byte { - return cdc.MustMarshalBinary(pk) + return cdc.MustMarshalBinaryBare(pk) } // Address returns tmhash(ThresholdMultiSignaturePubKey.Bytes()) diff --git a/crypto/multisig/threshold_multisig_test.go b/crypto/multisig/threshold_multisig_test.go index 6036554a3..2d2fdeec8 100644 --- a/crypto/multisig/threshold_multisig_test.go +++ b/crypto/multisig/threshold_multisig_test.go @@ -83,7 +83,7 @@ func TestMultiSigPubkeyEquality(t *testing.T) { pubkeys, _ := generatePubKeysAndSignatures(5, msg) multisigKey := NewThresholdMultiSignaturePubKey(2, pubkeys) var unmarshalledMultisig *ThresholdMultiSignaturePubKey - cdc.MustUnmarshalBinary(multisigKey.Bytes(), &unmarshalledMultisig) + cdc.MustUnmarshalBinaryBare(multisigKey.Bytes(), &unmarshalledMultisig) require.True(t, multisigKey.Equals(unmarshalledMultisig)) // Ensure that reordering pubkeys is treated as a different pubkey From 6beaf6e72dd2606c74d417334f2b502da8b27421 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Mon, 13 Aug 2018 18:46:33 -0700 Subject: [PATCH 09/10] (squash this) address Jae's comments on `NumTrueBitsBefore` --- crypto/multisig/compact_bit_array.go | 10 ++++------ crypto/multisig/multisignature.go | 2 +- crypto/multisig/threshold_multisig.go | 2 +- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/crypto/multisig/compact_bit_array.go b/crypto/multisig/compact_bit_array.go index be31a5c70..94e9791cd 100644 --- a/crypto/multisig/compact_bit_array.go +++ b/crypto/multisig/compact_bit_array.go @@ -71,12 +71,10 @@ func (bA *CompactBitArray) SetIndex(i int, v bool) bool { return true } -// NumOfTrueBitsBefore returns the location of the given index, among the -// values in the bit array that are set to true. -// e.g. if bA = _XX_X_X, NumOfTrueBitsBefore(4) = 2, since -// the value at index 4 of the bit array is the third -// value that is true. (And it is 0-indexed) -func (bA *CompactBitArray) NumOfTrueBitsBefore(index int) int { +// NumTrueBitsBefore returns the number of bits set to true before the +// given index. e.g. if bA = _XX__XX, NumOfTrueBitsBefore(4) = 2, since +// there are two bits set to true before index 4. +func (bA *CompactBitArray) NumTrueBitsBefore(index int) int { numTrueValues := 0 for i := 0; i < index; i++ { if bA.GetIndex(i) { diff --git a/crypto/multisig/multisignature.go b/crypto/multisig/multisignature.go index 374b5e1b4..29e8a30b9 100644 --- a/crypto/multisig/multisignature.go +++ b/crypto/multisig/multisignature.go @@ -33,7 +33,7 @@ func getIndex(pk crypto.PubKey, keys []crypto.PubKey) int { // AddSignature adds a signature to the multisig, at the corresponding index. // If the signature already exists, replace it. func (mSig *Multisignature) AddSignature(sig []byte, index int) { - newSigIndex := mSig.BitArray.NumOfTrueBitsBefore(index) + newSigIndex := mSig.BitArray.NumTrueBitsBefore(index) // Signature already exists, just replace the value there if mSig.BitArray.GetIndex(index) { mSig.Sigs[newSigIndex] = sig diff --git a/crypto/multisig/threshold_multisig.go b/crypto/multisig/threshold_multisig.go index 01462d6fb..58a3ea0e3 100644 --- a/crypto/multisig/threshold_multisig.go +++ b/crypto/multisig/threshold_multisig.go @@ -47,7 +47,7 @@ func (pk *ThresholdMultiSignaturePubKey) VerifyBytes(msg []byte, marshalledSig [ return false } // ensure at least k signatures are set - if sig.BitArray.NumOfTrueBitsBefore(size) < int(pk.K) { + if sig.BitArray.NumTrueBitsBefore(size) < int(pk.K) { return false } // index in the list of signatures which we are concerned with. From 2fe34491ba7c6d90b11f0ac2b6261bc4e8c318b1 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Tue, 14 Aug 2018 11:42:40 -0700 Subject: [PATCH 10/10] (squash this) Fix build errors --- crypto/multisig/compact_bit_array_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crypto/multisig/compact_bit_array_test.go b/crypto/multisig/compact_bit_array_test.go index 3382c8b93..8b342b7ad 100644 --- a/crypto/multisig/compact_bit_array_test.go +++ b/crypto/multisig/compact_bit_array_test.go @@ -170,7 +170,7 @@ func TestCompactBitArrayNumOfTrueBitsBefore(t *testing.T) { require.NoError(t, err) for i := 0; i < len(tc.bAIndex); i++ { - require.Equal(t, tc.trueValueIndex[i], bA.NumOfTrueBitsBefore(tc.bAIndex[i]), "tc %d, i %d", tcIndex, i) + require.Equal(t, tc.trueValueIndex[i], bA.NumTrueBitsBefore(tc.bAIndex[i]), "tc %d, i %d", tcIndex, i) } }) }