Skip to content

Commit

Permalink
Fixed race condition in Commit; fixed wrong cast check to BLS pub key
Browse files Browse the repository at this point in the history
  • Loading branch information
sergio-mena committed Jan 11, 2025
1 parent 9739076 commit 6266c88
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 8 deletions.
7 changes: 4 additions & 3 deletions internal/consensus/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -1330,7 +1330,8 @@ func (cs *State) createProposalBlock(ctx context.Context) (*types.Block, error)
case ok && lastCommitAsVs.HasTwoThirdsMajority():
// Make the commit from LastCommit.
_, blsKey := cs.privValidatorPubKey.(*bls12381.PubKey)
canBeAggregated := blsKey &&
_, blsKey2 := cs.privValidatorPubKey.(bls12381.PubKey)
canBeAggregated := (blsKey || blsKey2) &&
cs.state.Validators.AllKeysHaveSameType()
if canBeAggregated {
if !cs.isPBTSEnabled(cs.Height) {
Expand Down Expand Up @@ -2604,13 +2605,13 @@ func (cs *State) AddCommit(commit *types.Commit, peerID p2p.ID) (added bool, err
extEnabled := cs.isVoteExtensionsEnabled(commit.Height)
if extEnabled {
// We don't support receiving commits with vote extensions enabled ATM.
cs.Logger.Error("Received commit with vote extension for height %v (extensions disabled) from peer ID %s", commit.Height, peerID)
cs.Logger.Error("Received commit with vote extensions enabled", "height", commit.Height, "peer_ID", peerID)
return added, err
}

if !commit.HasAggregatedSignature() {
// Only accept aggregated commits
cs.Logger.Error("Received non aggregated commit for height %v from peer ID %s", commit.Height, peerID)
cs.Logger.Error("Received non aggregated commit", "commit", commit.Height, "peer_ID", peerID, "commit", commit)
return added, err
}
// No need to check blockID. If the commit is valid, 2/3 of the voting power has signed it.
Expand Down
22 changes: 17 additions & 5 deletions types/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -865,11 +865,12 @@ type Commit struct {
BlockID BlockID `json:"block_id"`
Signatures []CommitSig `json:"signatures"`

bitArray *bits.BitArray
// Memoized in first call to corresponding method.
// NOTE: can't memoize in constructor because constructor isn't used for
// unmarshaling.
hash cmtbytes.HexBytes
hash cmtbytes.HexBytes
bitArray *bits.BitArray
mtx cmtsync.Mutex // Protects hash and bitArray.
}

var _ VoteSetReader = (*Commit)(nil)
Expand All @@ -878,8 +879,12 @@ var _ VoteSetReader = (*Commit)(nil)
func (commit *Commit) Clone() *Commit {
sigs := make([]CommitSig, len(commit.Signatures))
copy(sigs, commit.Signatures)
commCopy := *commit
commCopy.Signatures = sigs
commCopy := Commit{
Height: commit.Height,
Round: commit.Round,
BlockID: commit.BlockID,
Signatures: sigs,
}
return &commCopy
}

Expand Down Expand Up @@ -909,6 +914,8 @@ func (commit *Commit) Size() int {
// this extended commit.
// Implements VoteSetReader.
func (commit *Commit) BitArray() *bits.BitArray {
commit.mtx.Lock()
defer commit.mtx.Unlock()
if commit.bitArray == nil {
initialBitFn := func(i int) bool {
// TODO: need to check the BlockID otherwise we could be counting conflicts,
Expand Down Expand Up @@ -1010,6 +1017,8 @@ func (commit *Commit) Hash() cmtbytes.HexBytes {
if commit == nil {
return nil
}
commit.mtx.Lock()
defer commit.mtx.Unlock()
if commit.hash == nil {
bs := make([][]byte, len(commit.Signatures))
for i, commitSig := range commit.Signatures {
Expand Down Expand Up @@ -1055,6 +1064,9 @@ func (commit *Commit) StringIndented(indent string) string {
for i, commitSig := range commit.Signatures {
commitSigStrings[i] = commitSig.String()
}
commit.mtx.Lock()
hash := commit.hash
commit.mtx.Unlock()
return fmt.Sprintf(`Commit{
%s Height: %d
%s Round: %d
Expand All @@ -1067,7 +1079,7 @@ func (commit *Commit) StringIndented(indent string) string {
indent, commit.BlockID,
indent,
indent, strings.Join(commitSigStrings, "\n"+indent+" "),
indent, commit.hash)
indent, hash)
}

// ToProto converts Commit to protobuf.
Expand Down

0 comments on commit 6266c88

Please sign in to comment.