fix: general improvements in tm2/bft/consensus

tm2/ordering/ordering.go

@@ -0,0 +1,35 @@
+package ordering
+
+type Order int
+
+const (
+	less    Order = -1
+	equal   Order = 0
+	greater Order = 1
+)
+
+var (
+	Less    = Ordering{less}
+	Equal   = Ordering{equal}
+	Greater = Ordering{greater}
+)
+
+type Ordering struct {
+	value Order
+}
+
+func NewOrdering(order Order) Ordering {
+	return Ordering{value: order}
+}
+
+func (o Ordering) IsEqual() bool {
+	return o == Equal
+}
+
+func (o Ordering) IsLess() bool {
+	return o == Less
+}
+
+func (o Ordering) IsGreater() bool {
+	return o == Greater
+}

tm2/pkg/bft/consensus/common_test.go

@@ -127,7 +127,7 @@ func (vss ValidatorStubsByAddress) Len() int {
 }
 
 func (vss ValidatorStubsByAddress) Less(i, j int) bool {
-	return vss[i].GetPubKey().Address().Compare(vss[j].GetPubKey().Address()) == -1
+	return vss[i].GetPubKey().Address().Compare(vss[j].GetPubKey().Address()).IsLess()
 }
 
 func (vss ValidatorStubsByAddress) Swap(i, j int) {

tm2/pkg/bft/consensus/reactor.go

@@ -243,9 +243,9 @@ func (conR *ConsensusReactor) Receive(chID byte, src p2p.Peer, msgBytes []byte)
 			ps.ApplyHasVoteMessage(msg)
 		case *VoteSetMaj23Message:
 			cs := conR.conS
-			cs.mtx.Lock()
+			cs.mtx.RLock() // must not starve
 			height, votes := cs.Height, cs.Votes
-			cs.mtx.Unlock()
+			cs.mtx.RUnlock()
 			if height != msg.Height {
 				return
 			}
@@ -325,9 +325,9 @@ func (conR *ConsensusReactor) Receive(chID byte, src p2p.Peer, msgBytes []byte)
 		switch msg := msg.(type) {
 		case *VoteSetBitsMessage:
 			cs := conR.conS
-			cs.mtx.Lock()
+			cs.mtx.RLock()
 			height, votes := cs.Height, cs.Votes
-			cs.mtx.Unlock()
+			cs.mtx.RUnlock()
 
 			if height == msg.Height {
 				var ourVotes *bitarray.BitArray

tm2/pkg/bft/consensus/state.go

@@ -69,9 +69,8 @@ func (ti *timeoutInfo) String() string {
 func (ti *timeoutInfo) GetHRS() cstypes.HRS {
 	if ti == nil {
 		return cstypes.HRS{}
-	} else {
-		return cstypes.HRS{ti.Height, ti.Round, ti.Step}
 	}
+	return cstypes.HRS{ti.Height, ti.Round, ti.Step}
 }
 
 // interface to the mempool

tm2/pkg/bft/consensus/types/round_state.go

@@ -95,23 +95,6 @@ type HRS struct {
 	Step   RoundStepType `json:"step"`
 }
 
-func (hrs HRS) Compare(other HRS) int {
-	if hrs.Height < other.Height {
-		return -1
-	} else if hrs.Height == other.Height {
-		if hrs.Round < other.Round {
-			return -1
-		} else if hrs.Round == other.Round {
-			if hrs.Step < other.Step {
-				return -1
-			} else if hrs.Step == other.Step {
-				return 0
-			}
-		}
-	}
-	return 1
-}
-
 func (hrs HRS) IsHRSZero() bool {
 	return hrs == HRS{}
 }

tm2/pkg/bft/types/priv_validator.go

@@ -29,13 +29,11 @@ func (pvs PrivValidatorsByAddress) Len() int {
 
 func (pvs PrivValidatorsByAddress) Less(i, j int) bool {
 	return pvs[i].GetPubKey().Address().Compare(
-		pvs[j].GetPubKey().Address()) == -1
+		pvs[j].GetPubKey().Address()).IsLess()
 }
 
 func (pvs PrivValidatorsByAddress) Swap(i, j int) {
-	it := pvs[i]
-	pvs[i] = pvs[j]
-	pvs[j] = it
+	pvs[i], pvs[j] = pvs[j], pvs[i]
 }
 
 // ----------------------------------------

tm2/pkg/bft/types/validator.go

@@ -3,6 +3,8 @@ package types
 import (
 	"fmt"
 
+	"github.com/gnolang/gno/tm2/ordering"
+
 	abci "github.com/gnolang/gno/tm2/pkg/bft/abci/types"
 	"github.com/gnolang/gno/tm2/pkg/crypto"
 	"github.com/gnolang/gno/tm2/pkg/random"
@@ -57,16 +59,16 @@ func (v *Validator) CompareProposerPriority(other *Validator) *Validator {
 		return v
 	case v.ProposerPriority < other.ProposerPriority:
 		return other
+	}
+
+	result := v.Address.Compare(other.Address)
+	switch result {
+	case ordering.Less:
+		return v
+	case ordering.Greater:
+		return other
 	default:
-		result := v.Address.Compare(other.Address)
-		switch {
-		case result < 0:
-			return v
-		case result > 0:
-			return other
-		default:
-			panic("Cannot compare identical validators")
-		}
+		panic("Cannot compare identical validators")
 	}
 }
 

tm2/pkg/bft/types/validator_set.go

@@ -223,7 +223,8 @@ func (vals *ValidatorSet) Copy() *ValidatorSet {
 // otherwise.
 func (vals *ValidatorSet) HasAddress(address crypto.Address) bool {
 	idx := sort.Search(len(vals.Validators), func(i int) bool {
-		return address.Compare(vals.Validators[i].Address) <= 0
+		cmp := address.Compare(vals.Validators[i].Address)
+		return cmp.IsLess() || cmp.IsEqual()
 	})
 	return idx < len(vals.Validators) && vals.Validators[idx].Address == address
 }
@@ -232,7 +233,8 @@ func (vals *ValidatorSet) HasAddress(address crypto.Address) bool {
 // itself if found. Otherwise, -1 and nil are returned.
 func (vals *ValidatorSet) GetByAddress(address crypto.Address) (index int, val *Validator) {
 	idx := sort.Search(len(vals.Validators), func(i int) bool {
-		return address.Compare(vals.Validators[i].Address) <= 0
+		cmp := address.Compare(vals.Validators[i].Address)
+		return cmp.IsLess() || cmp.IsEqual()
 	})
 	if idx < len(vals.Validators) && vals.Validators[idx].Address == address {
 		return idx, vals.Validators[idx].Copy()
@@ -440,7 +442,7 @@ func (vals *ValidatorSet) applyUpdates(updates []*Validator) {
 	i := 0
 
 	for len(existing) > 0 && len(updates) > 0 {
-		if existing[0].Address.Compare(updates[0].Address) < 0 { // unchanged validator
+		if existing[0].Address.Compare(updates[0].Address).IsLess() { // unchanged validator
 			merged[i] = existing[0]
 			existing = existing[1:]
 		} else {
@@ -813,13 +815,11 @@ func (valz ValidatorsByAddress) Len() int {
 }
 
 func (valz ValidatorsByAddress) Less(i, j int) bool {
-	return valz[i].Address.Compare(valz[j].Address) == -1
+	return valz[i].Address.Compare(valz[j].Address).IsLess()
 }
 
 func (valz ValidatorsByAddress) Swap(i, j int) {
-	it := valz[i]
-	valz[i] = valz[j]
-	valz[j] = it
+	valz[i], valz[j] = valz[j], valz[i]
 }
 
 // ----------------------------------------

tm2/pkg/bft/types/validator_set_test.go

@@ -1319,7 +1319,7 @@ func (valz validatorsByPriority) Less(i, j int) bool {
 	if valz[i].ProposerPriority > valz[j].ProposerPriority {
 		return false
 	}
-	return valz[i].Address.Compare(valz[j].Address) < 0
+	return valz[i].Address.Compare(valz[j].Address).IsLess()
 }
 
 func (valz validatorsByPriority) Swap(i, j int) {

tm2/pkg/bft/wal/wal.go

@@ -500,6 +500,7 @@ func (enc *WALWriter) Write(v TimedWALMessage) error {
 	line64 := base64stdnp.EncodeToString(line)
 	line64 += "\n"
 	_, err := enc.wr.Write([]byte(line64))
+
 	return err
 }
 

tm2/pkg/crypto/crypto.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 
+	"github.com/gnolang/gno/tm2/ordering"
 	"github.com/gnolang/gno/tm2/pkg/bech32"
 	"github.com/gnolang/gno/tm2/pkg/crypto/tmhash"
 )
@@ -69,12 +70,8 @@ func (addr *Address) UnmarshalAmino(b32str string) (err error) {
 	return nil
 }
 
-func (addr Address) Compare(other Address) int {
-	bz1 := make([]byte, len(addr))
-	bz2 := make([]byte, len(other))
-	copy(bz1, addr[:])
-	copy(bz2, other[:])
-	return bytes.Compare(bz1, bz2)
+func (addr Address) Compare(other Address) ordering.Ordering {
+	return ordering.NewOrdering(ordering.Order(bytes.Compare(addr[:], other[:])))
 }
 
 func (addr Address) IsZero() bool {

tm2/pkg/crypto/keys/client/add.go

@@ -187,7 +187,7 @@ func execAdd(cfg *AddCfg, args []string, io commands.IO) error {
 			// Handle --nosort
 			if !cfg.NoSort {
 				sort.Slice(pks, func(i, j int) bool {
-					return pks[i].Address().Compare(pks[j].Address()) < 0
+					return pks[i].Address().Compare(pks[j].Address()).IsLess()
 				})
 			}
 

tm2/pkg/store/cache/mergeiterator.go

@@ -3,6 +3,7 @@ package cache
 import (
 	"bytes"
 
+	"github.com/gnolang/gno/tm2/ordering"
 	"github.com/gnolang/gno/tm2/pkg/store/types"
 )
 
@@ -35,12 +36,12 @@ func newCacheMergeIterator(parent, cache types.Iterator, ascending bool) *cacheM
 func (iter *cacheMergeIterator) Domain() (start, end []byte) {
 	startP, endP := iter.parent.Domain()
 	startC, endC := iter.cache.Domain()
-	if iter.compare(startP, startC) < 0 {
+	if iter.compare(startP, startC).IsLess() {
 		start = startP
 	} else {
 		start = startC
 	}
-	if iter.compare(endP, endC) < 0 {
+	if iter.compare(endP, endC).IsLess() {
 		end = endC
 	} else {
 		end = endP
@@ -73,12 +74,12 @@ func (iter *cacheMergeIterator) Next() {
 	// Both are valid.  Compare keys.
 	keyP, keyC := iter.parent.Key(), iter.cache.Key()
 	switch iter.compare(keyP, keyC) {
-	case -1: // parent < cache
+	case ordering.Less: // parent < cache
 		iter.parent.Next()
-	case 0: // parent == cache
+	case ordering.Equal: // parent == cache
 		iter.parent.Next()
 		iter.cache.Next()
-	case 1: // parent > cache
+	case ordering.Greater: // parent > cache
 		iter.cache.Next()
 	}
 }
@@ -102,11 +103,11 @@ func (iter *cacheMergeIterator) Key() []byte {
 	keyP, keyC := iter.parent.Key(), iter.cache.Key()
 	cmp := iter.compare(keyP, keyC)
 	switch cmp {
-	case -1: // parent < cache
+	case ordering.Less: // parent < cache
 		return keyP
-	case 0: // parent == cache
+	case ordering.Equal: // parent == cache
 		return keyP
-	case 1: // parent > cache
+	case ordering.Greater: // parent > cache
 		return keyC
 	default:
 		panic("invalid compare result")
@@ -132,11 +133,11 @@ func (iter *cacheMergeIterator) Value() []byte {
 	keyP, keyC := iter.parent.Key(), iter.cache.Key()
 	cmp := iter.compare(keyP, keyC)
 	switch cmp {
-	case -1: // parent < cache
+	case ordering.Less: // parent < cache
 		return iter.parent.Value()
-	case 0: // parent == cache
+	case ordering.Equal: // parent == cache
 		return iter.cache.Value()
-	case 1: // parent > cache
+	case ordering.Greater: // parent > cache
 		return iter.cache.Value()
 	default:
 		panic("invalid comparison result")
@@ -150,11 +151,11 @@ func (iter *cacheMergeIterator) Close() {
 }
 
 // Like bytes.Compare but opposite if not ascending.
-func (iter *cacheMergeIterator) compare(a, b []byte) int {
+func (iter *cacheMergeIterator) compare(a, b []byte) ordering.Ordering {
 	if iter.ascending {
-		return bytes.Compare(a, b)
+		return ordering.NewOrdering(ordering.Order(bytes.Compare(a, b)))
 	}
-	return bytes.Compare(a, b) * -1
+	return ordering.NewOrdering(ordering.Order(bytes.Compare(a, b) * -1))
 }
 
 // Skip all delete-items from the cache w/ `key < until`.  After this function,
@@ -165,7 +166,7 @@ func (iter *cacheMergeIterator) compare(a, b []byte) int {
 func (iter *cacheMergeIterator) skipCacheDeletes(until []byte) {
 	for iter.cache.Valid() &&
 		iter.cache.Value() == nil &&
-		(until == nil || iter.compare(iter.cache.Key(), until) < 0) {
+		(until == nil || iter.compare(iter.cache.Key(), until).IsLess()) {
 		iter.cache.Next()
 	}
 }
@@ -191,10 +192,10 @@ func (iter *cacheMergeIterator) skipUntilExistsOrInvalid() bool {
 		keyP := iter.parent.Key()
 		keyC := iter.cache.Key()
 		switch iter.compare(keyP, keyC) {
-		case -1: // parent < cache.
+		case ordering.Less: // parent < cache.
 			return true
 
-		case 0: // parent == cache.
+		case ordering.Equal: // parent == cache.
 
 			// Skip over if cache item is a delete.
 			valueC := iter.cache.Value()
@@ -207,7 +208,7 @@ func (iter *cacheMergeIterator) skipUntilExistsOrInvalid() bool {
 
 			return true // cache exists.
 
-		case 1: // cache < parent
+		case ordering.Greater: // cache < parent
 
 			// Skip over if cache item is a delete.
 			valueC := iter.cache.Value()