Skip to content

Commit

Permalink
fix: adding committers to the certificate (#623)
Browse files Browse the repository at this point in the history
  • Loading branch information
b00f authored Aug 7, 2023
1 parent 3025018 commit cac4908
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 50 deletions.
11 changes: 4 additions & 7 deletions types/block/block_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
49 changes: 19 additions & 30 deletions types/block/certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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.
Expand Down
13 changes: 0 additions & 13 deletions types/block/certificate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}

0 comments on commit cac4908

Please sign in to comment.