feat: Modify MakeCommit and AggregateSignature to one operation at once

consensus/state.go

@@ -1101,7 +1101,6 @@ func (cs *State) createProposalBlock(round int) (block *types.Block, blockParts
  case cs.LastCommit.HasTwoThirdsMajority():
  // Make the commit from LastCommit
  commit = cs.LastCommit.MakeCommit()
- commit.AggregateSignatures()
  default: // This shouldn't happen.
  cs.Logger.Error("enterPropose: Cannot propose anything: No commit for the previous block")
  return

types/block_test.go

@@ -9,6 +9,7 @@ import (
  "math"
  "os"
  "reflect"
+ "strings"
  "testing"
  "time"
 
@@ -288,7 +289,12 @@ func TestCommit(t *testing.T) {
  require.NotNil(t, commit.BitArray())
  assert.Equal(t, bits.NewBitArray(10).Size(), commit.BitArray().Size())
 
- assert.Equal(t, voteSet.GetByIndex(0), commit.GetByIndex(0))
+ if len(voteSet.GetByIndex(0).Signature) != bls.SignatureSize {
+ assert.Equal(t, voteSet.GetByIndex(0), commit.GetByIndex(0))
+ } else {
+ assert.NotNil(t, commit.AggregatedSignature)
+ isEqualVoteWithoutSignature(t, voteSet.GetByIndex(0), commit.GetByIndex(0))
+ }
 }
 
 func TestCommitValidateBasic(t *testing.T) {
@@ -800,23 +806,65 @@ func TestCommitToVoteSet(t *testing.T) {
  lastID := makeBlockIDRandom()
  h := int64(3)
 
- voteSet, _, valSet, vals := randVoteSet(h-1, 1, PrecommitType, 10, 1)
+ voteSet, _, voterSet, vals := randVoteSet(h-1, 1, PrecommitType, 10, 1)
  commit, err := MakeCommit(lastID, h-1, 1, voteSet, vals, time.Now())
  assert.NoError(t, err)
 
  chainID := voteSet.ChainID()
- voteSet2 := CommitToVoteSet(chainID, commit, valSet)
-
+ voteSet2 := CommitToVoteSet(chainID, commit, voterSet)
  for i := 0; i < len(vals); i++ {
+ // This is the vote before `MakeCommit`.
  vote1 := voteSet.GetByIndex(i)
+ // This is the vote created from `CommitToVoteSet`
  vote2 := voteSet2.GetByIndex(i)
+ // This is the vote created from `MakeCommit`
  vote3 := commit.GetVote(i)
 
- vote1bz := cdc.MustMarshalBinaryBare(vote1)
- vote2bz := cdc.MustMarshalBinaryBare(vote2)
- vote3bz := cdc.MustMarshalBinaryBare(vote3)
- assert.Equal(t, vote1bz, vote2bz)
- assert.Equal(t, vote1bz, vote3bz)
+ if len(vote1.Signature) != bls.SignatureSize {
+ vote1bz := cdc.MustMarshalBinaryBare(vote1)
+ vote2bz := cdc.MustMarshalBinaryBare(vote2)
+ vote3bz := cdc.MustMarshalBinaryBare(vote3)
+ assert.Equal(t, vote1bz, vote2bz)
+ assert.Equal(t, vote1bz, vote3bz)
+ } else {
+ vote2bz := cdc.MustMarshalBinaryBare(vote2)
+ vote3bz := cdc.MustMarshalBinaryBare(vote3)
+ assert.Equal(t, vote2bz, vote3bz)
+ assert.NotNil(t, commit.AggregatedSignature)
+ assert.Nil(t, vote2.Signature)
+ assert.Nil(t, vote3.Signature)
+ isEqualVoteWithoutSignature(t, vote1, vote2)
+ isEqualVoteWithoutSignature(t, vote1, vote3)
+ }
+ }
+}
+
+func TestMakeCommitPanicByAggregatedCommitAndVoteSet(t *testing.T) {
+ lastID := makeBlockIDRandom()
+ h := int64(3)
+
+ voteSet, _, voterSet, vals := randVoteSet(h-1, 1, PrecommitType, 10, 1)
+ commit, err := MakeCommit(lastID, h-1, 1, voteSet, vals, time.Now())
+ assert.NoError(t, err)
+
+ chainID := voteSet.ChainID()
+ voteSet2 := CommitToVoteSet(chainID, commit, voterSet)
+
+ // panic test
+ defer func() {
+ err := recover()
+ if err != nil {
+ wantStr := "This signature of commitSig is already aggregated: commitSig: <"
+ gotStr := fmt.Sprintf("%v", err)
+ isPanic := strings.Contains(gotStr, wantStr)
+ assert.True(t, isPanic)
+ } else {
+ t.Log("Not aggregated")
+ }
+ }()
+ if commit.AggregatedSignature != nil {
+ voteSet2.MakeCommit()
+ assert.Fail(t, "Don't reach here")
  }
 }
 

types/vote_set.go

@@ -586,14 +586,19 @@ func (voteSet *VoteSet) MakeCommit() *Commit {
  commitSigs := make([]CommitSig, len(voteSet.votes))
  for i, v := range voteSet.votes {
  commitSig := v.CommitSig()
+ if !commitSig.Absent() && commitSig.Signature == nil {
+ panic(fmt.Sprintf("This signature of commitSig is already aggregated: commitSig: <%v>", commitSig))
+ }
  // if block ID exists but doesn't match, exclude sig
  if commitSig.ForBlock() && !v.BlockID.Equals(*voteSet.maj23) {
  commitSig = NewCommitSigAbsent()
  }
  commitSigs[i] = commitSig
  }
+ newCommit := NewCommit(voteSet.GetHeight(), voteSet.GetRound(), *voteSet.maj23, commitSigs)
+ newCommit.AggregateSignatures()
 
- return NewCommit(voteSet.GetHeight(), voteSet.GetRound(), *voteSet.maj23, commitSigs)
+ return newCommit
 }
 
 //--------------------------------------------------------------------------------

types/vote_set_test.go

@@ -649,7 +649,6 @@ func TestMakeCommit(t *testing.T) {
  }
 
  commit := voteSet.MakeCommit()
- commit.AggregateSignatures()
 
  assert.Equal(t, height, commit.Height)
  assert.Equal(t, round, commit.Round)

types/vote_test.go

@@ -44,6 +44,16 @@ func exampleVote(t byte) *Vote {
  }
 }
 
+func isEqualVoteWithoutSignature(t *testing.T, vote1, vote2 *Vote) {
+ assert.Equal(t, vote1.Type, vote2.Type)
+ assert.Equal(t, vote1.Height, vote2.Height)
+ assert.Equal(t, vote1.Round, vote2.Round)
+ assert.Equal(t, vote1.BlockID, vote2.BlockID)
+ assert.Equal(t, vote1.Timestamp, vote2.Timestamp)
+ assert.Equal(t, vote1.ValidatorAddress, vote2.ValidatorAddress)
+ assert.Equal(t, vote1.ValidatorIndex, vote2.ValidatorIndex)
+}
+
 func TestVoteSignable(t *testing.T) {
  vote := examplePrecommit()
  signBytes := vote.SignBytes("test_chain_id")