From cac4908d338d1a408801eb2db3b8887ba65196bd Mon Sep 17 00:00:00 2001
From: b00f <mostafa.sedaghat@gmail.com>
Date: Mon, 7 Aug 2023 13:52:53 +0800
Subject: [PATCH] fix: adding committers to the certificate (#623)

---
 types/block/block_test.go       | 11 +++-----
 types/block/certificate.go      | 49 +++++++++++++--------------------
 types/block/certificate_test.go | 13 ---------
 3 files changed, 23 insertions(+), 50 deletions(-)

diff --git a/types/block/block_test.go b/types/block/block_test.go
index 9a8438532..0f3e681f0 100644
--- a/types/block/block_test.go
+++ b/types/block/block_test.go
@@ -180,11 +180,8 @@ func TestBlockHash(t *testing.T) {
 
 	headerSize := b.Header().SerializeSize()
 	headerData := d[:headerSize]
-	// TODO: uncomment this comment later
-	// TODO: make HashBytes unexported
-	// certSize := b.PrevCertificate().SerializeSize()
-	// certData := d[headerSize : headerSize+certSize]
-	certData := b.PrevCertificate().HashBytes()
+	certSize := b.PrevCertificate().SerializeSize()
+	certData := d[headerSize : headerSize+certSize]
 	certHash := hash.CalcHash(certData)
 
 	txHashes := make([]hash.Hash, 0)
@@ -199,10 +196,10 @@ func TestBlockHash(t *testing.T) {
 	hashData = append(hashData, util.Int32ToSlice(int32(b.Transactions().Len()))...)
 
 	expected1 := hash.CalcHash(hashData)
-	expected2, _ := hash.FromString("aa0244a7464e2d7318d0f0bf40218fcd5cc0a0d8746e29348666160e6b58a20c")
+	expected2, _ := hash.FromString("285ed60d44c0650e27fb25d5adc4b2cc5e35a2ba61f9c2f9d10f2de0251aee45")
 	assert.Equal(t, b.Hash(), expected1)
 	assert.Equal(t, b.Hash(), expected2)
-	assert.Equal(t, b.Stamp(), hash.Stamp{0xaa, 0x02, 0x44, 0xa7})
+	assert.Equal(t, b.Stamp(), hash.Stamp{0x28, 0x5e, 0xd6, 0x0d})
 }
 
 func TestMakeBlock(t *testing.T) {
diff --git a/types/block/certificate.go b/types/block/certificate.go
index 6404a8d46..3abdf2245 100644
--- a/types/block/certificate.go
+++ b/types/block/certificate.go
@@ -35,10 +35,21 @@ func NewCertificate(round int16, committers, absentees []int32, signature *bls.S
 	return cert
 }
 
-func (cert *Certificate) Round() int16              { return cert.data.Round }
-func (cert *Certificate) Committers() []int32       { return cert.data.Committers }
-func (cert *Certificate) Absentees() []int32        { return cert.data.Absentees }
-func (cert *Certificate) Signature() *bls.Signature { return cert.data.Signature }
+func (cert *Certificate) Round() int16 {
+	return cert.data.Round
+}
+
+func (cert *Certificate) Committers() []int32 {
+	return cert.data.Committers
+}
+
+func (cert *Certificate) Absentees() []int32 {
+	return cert.data.Absentees
+}
+
+func (cert *Certificate) Signature() *bls.Signature {
+	return cert.data.Signature
+}
 
 func (cert *Certificate) SanityCheck() error {
 	if cert.Round() < 0 {
@@ -61,35 +72,13 @@ func (cert *Certificate) SanityCheck() error {
 	return nil
 }
 
-// Remove this function later
-// read below comment
-func (cert *Certificate) HashBytes() []byte {
+func (cert *Certificate) Hash() hash.Hash {
 	w := bytes.NewBuffer(make([]byte, 0, cert.SerializeSize()))
-	if err := encoding.WriteVarInt(w, uint64(cert.Round())); err != nil {
-		return nil
+	if err := cert.Encode(w); err != nil {
+		return hash.UndefHash
 	}
-	if err := encoding.WriteVarInt(w, uint64(len(cert.data.Absentees))); err != nil {
-		return nil
-	}
-	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())
+	return hash.CalcHash(w.Bytes())
 }
 
 // SerializeSize returns the number of bytes it would take to serialize the block.
diff --git a/types/block/certificate_test.go b/types/block/certificate_test.go
index 0030c4029..2f0fde365 100644
--- a/types/block/certificate_test.go
+++ b/types/block/certificate_test.go
@@ -109,16 +109,3 @@ 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) {
-	ts := testsuite.NewTestSuite(t)
-
-	temp := ts.GenerateTestCertificate(ts.RandomHash())
-	cert1 := block.NewCertificate(temp.Round(), []int32{1, 2, 3, 4}, []int32{2}, temp.Signature())
-	cert2 := block.NewCertificate(temp.Round(), []int32{1, 2, 3, 4, 5}, []int32{2}, temp.Signature())
-
-	assert.Equal(t, cert1.Hash(), cert2.Hash())
-}