fix: removing committers from the certificate hash (#444)

* fix: removing committers from the certificate hash

* tests: fixing a broken test

types/block/block_test.go

@@ -113,9 +113,11 @@ func TestBlockHash(t *testing.T) {
 	assert.Equal(t, d, d2)
 
 	headerSize := b.Header().SerializeSize()
-	certSize := b.PrevCertificate().SerializeSize()
 	headerData := d[:headerSize]
-	certData := d[headerSize : headerSize+certSize]
+	// uncomment this comment later
+	// certSize := b.PrevCertificate().SerializeSize()
+	// certData := d[headerSize : headerSize+certSize]
+	certData := b.PrevCertificate().hashBytes()
 	certHash := hash.CalcHash(certData)
 
 	txHashes := make([]hash.Hash, 0)
@@ -130,10 +132,10 @@ func TestBlockHash(t *testing.T) {
 	hashData = append(hashData, util.Int32ToSlice(int32(b.Transactions().Len()))...)
 
 	expected1 := hash.CalcHash(hashData)
-	expected2, _ := hash.FromString("c856bd7d0ce99842ec450ffad6416c45320d667b8b7c4927727860a43290595c")
+	expected2, _ := hash.FromString("665e48e4dcbe3d4ad4871ed01606ced559501ad969093f9c63f8f0c41b25819b")
 	assert.Equal(t, b.Hash(), expected1)
 	assert.Equal(t, b.Hash(), expected2)
-	assert.Equal(t, b.Stamp(), hash.Stamp{0xc8, 0x56, 0xbd, 0x7d})
+	assert.Equal(t, b.Stamp(), hash.Stamp{0x66, 0x5e, 0x48, 0xe4})
 }
 
 func TestMakeBlock(t *testing.T) {

types/block/certificate.go

@@ -61,12 +61,35 @@ func (cert *Certificate) SanityCheck() error {
 	return nil
 }
 
-func (cert *Certificate) Hash() hash.Hash {
+// Remove this function later
+// read below comment
+func (cert *Certificate) hashBytes() []byte {
 	w := bytes.NewBuffer(make([]byte, 0, cert.SerializeSize()))
-	if err := cert.Encode(w); err != nil {
-		return hash.UndefHash
+	if err := encoding.WriteVarInt(w, uint64(cert.Round())); err != nil {
+		return nil
+	}
+	if err := encoding.WriteVarInt(w, uint64(len(cert.data.Absentees))); err != nil {
+		return nil
 	}
-	return hash.CalcHash(w.Bytes())
+	for _, n := range cert.data.Absentees {
+		if err := encoding.WriteVarInt(w, uint64(n)); err != nil {
+			return nil
+		}
+	}
+	if err := cert.data.Signature.Encode(w); err != nil {
+		return nil
+	}
+	return w.Bytes()
+}
+
+func (cert *Certificate) Hash() hash.Hash {
+	// TODO: Add a comment on certificate hash
+	// Technically, we don't need to include the committers list inside the certificate.
+	// At each height, the committers are the same as the committee members.
+	// As a possible enhancement in the future, we can remove the committers from the certificate.
+	// In this case, increasing the committee size won't increase the size of the certificate.
+
+	return hash.CalcHash(cert.hashBytes())
 }
 
 // SerializeSize returns the number of bytes it would take to serialize the block.

types/block/certificate_test.go

@@ -77,3 +77,15 @@ func TestCertificateHash(t *testing.T) {
 	assert.Equal(t, cert3.Absentees(), []int32{18})
 	assert.NoError(t, cert3.SanityCheck())
 }
+
+// This test ensures that committers are not part of the certificate hash
+// We can remove this tests if we remove the committers from the certificate
+// This test is not logical, since we have two certificate for the same block
+func TestCertificateHashWithoutCommitters(t *testing.T) {
+	temp := GenerateTestCertificate(hash.GenerateTestHash())
+
+	cert1 := NewCertificate(temp.Round(), []int32{1, 2, 3, 4}, []int32{2}, temp.Signature())
+	cert2 := NewCertificate(temp.Round(), []int32{1, 2, 3, 4, 5}, []int32{2}, temp.Signature())
+
+	assert.Equal(t, cert1.Hash(), cert2.Hash())
+}