Fix ticket handling, switch to exponential ticket election

gpbft/gpbft.go

@@ -7,7 +7,6 @@
 	"errors"
 	"fmt"
 	"math"
-	"math/big"
 	"slices"
 	"sort"
 	"time"
@@ -16,7 +15,6 @@
 	rlepluslazy "github.com/filecoin-project/go-bitfield/rle"
 	"github.com/filecoin-project/go-f3/merkle"
 	"go.opentelemetry.io/otel/metric"
-	"golang.org/x/crypto/blake2b"
 )
 
 type Phase uint8
@@ -378,7 +376,7 @@
 		// Receive each prefix of the proposal independently.
 		i.quality.ReceiveEachPrefix(msg.Sender, msg.Vote.Value)
 	case CONVERGE_PHASE:
-		if err := msgRound.converged.Receive(msg.Sender, msg.Vote.Value, msg.Ticket, msg.Justification); err != nil {
+		if err := msgRound.converged.Receive(msg.Sender, i.powerTable, msg.Vote.Value, msg.Ticket, msg.Justification); err != nil {
 			return false, fmt.Errorf("failed processing CONVERGE message: %w", err)
 		}
 	case PREPARE_PHASE:
@@ -437,7 +435,7 @@
 		return nil, nil, false
 	}
 	proposal := state.converged.FindMaxTicketProposal(i.powerTable)
-	if proposal.Justification == nil {
+	if !proposal.IsValid() {
 		// FindMaxTicketProposal returns a zero-valued ConvergeValue if no such ticket is
 		// found. Hence the check for nil. Otherwise, if found such ConvergeValue must
 		// have a non-nil justification.
@@ -547,7 +545,7 @@
 	}
 
 	winner := i.getRound(i.round).converged.FindMaxTicketProposal(i.powerTable)
-	if winner.Chain.IsZero() {
+	if !winner.IsValid() {
 		return fmt.Errorf("no values at CONVERGE")
 	}
 	possibleDecisionLastRound := i.getRound(i.round-1).committed.CouldReachStrongQuorumFor(
@@ -566,8 +564,8 @@
 		// Else preserve own proposal.
 		// This could alternatively loop to next lowest ticket as an optimisation to increase the
 		// chance of proposing the same value as other participants.
-		fallback, ok := i.getRound(i.round).converged.FindProposalFor(i.proposal)
-		if !ok {
+		fallback := i.getRound(i.round).converged.FindProposalFor(i.proposal)
+		if !fallback.IsValid() {
 			panic("own proposal not found at CONVERGE")
 		}
 		justification = fallback.Justification
@@ -1245,115 +1243,119 @@
 //// CONVERGE phase helper /////
 
 type convergeState struct {
-	// Stores this participant's value so the participant can use it even if it doesn't receive its own
-	// CONVERGE message (which carries the ticket) in a timely fashion.
-	self *ConvergeValue
 	// Participants from which a message has been received.
 	senders map[ActorID]struct{}
 	// Chains indexed by key.
 	values map[ChainKey]ConvergeValue
-	// Tickets provided by proposers of each chain.
-	tickets map[ChainKey][]ConvergeTicket
 }
 
+// ConvergeValue is valid when the Chain is non-zero and Justification is non-nil
 type ConvergeValue struct {
 	Chain         ECChain
 	Justification *Justification
+
+	Quality float64
 }
 
-type ConvergeTicket struct {
-	Sender ActorID
-	Ticket Ticket
+// TakeBetter merges the argument into the ConvergeValue if the ConvergeValue is zero valued or
+// if argument is better due to quality.
+func (cv *ConvergeValue) TakeBetter(cv2 ConvergeValue) {
+	if !cv.IsValid() || cv2.Quality < cv.Quality {
+		*cv = cv2
+	}
+}
+
+func (cv *ConvergeValue) IsValid() bool {
+	return !cv.Chain.IsZero() && cv.Justification != nil
 }
 
 func newConvergeState() *convergeState {
 	return &convergeState{
 		senders: map[ActorID]struct{}{},
 		values:  map[ChainKey]ConvergeValue{},
-		tickets: map[ChainKey][]ConvergeTicket{},
 	}
 }
 
 // SetSelfValue sets the participant's locally-proposed converge value.
 // This means the participant need not rely on messages broadcast to be received by itself.
 // See HasSelfValue.
 func (c *convergeState) SetSelfValue(value ECChain, justification *Justification) {
-	c.self = &ConvergeValue{
-		Chain:         value,
-		Justification: justification,
+	// any converge for the given value is better than self-reported
+	// as self-reported has no ticket
+	key := value.Key()
+	if _, ok := c.values[key]; !ok {
+		c.values[key] = ConvergeValue{
+			Chain:         value,
+			Justification: justification,
+			Quality:       math.Inf(1), // +Inf because any real ConvergeValue is better than self-value
+		}
 	}
 }
 
-// HasSelfValue checks whether the participant recorded a converge value.
-// See SetSelfValue.
-func (c *convergeState) HasSelfValue() bool {
-	return c.self != nil
-}
-
 // Receives a new CONVERGE value from a sender.
 // Ignores any subsequent value from a sender from which a value has already been received.
-func (c *convergeState) Receive(sender ActorID, value ECChain, ticket Ticket, justification *Justification) error {
+func (c *convergeState) Receive(sender ActorID, table PowerTable, value ECChain, ticket Ticket, justification *Justification) error {
 	if value.IsZero() {
 		return fmt.Errorf("bottom cannot be justified for CONVERGE")
 	}
+	if justification == nil {
+		return fmt.Errorf("CONVERGE message cannot carry nil-justification")
+	}
+
 	if _, ok := c.senders[sender]; ok {
 		return nil
 	}
 	c.senders[sender] = struct{}{}
-	key := value.Key()
+	senderPower, _ := table.Get(sender)
 
-	// Keep only the first justification and ticket received for a value.
-	if _, found := c.values[key]; !found {
-		c.values[key] = ConvergeValue{Chain: value, Justification: justification}
-		c.tickets[key] = append(c.tickets[key], ConvergeTicket{Sender: sender, Ticket: ticket})
+	key := value.Key()
+	// Keep only the first justification and best ticket
+	if v, found := c.values[key]; !found {
+		c.values[key] = ConvergeValue{
+			Chain:         value,
+			Justification: justification,
+			Quality:       ComputeTicketQuality(ticket, senderPower),
+		}
+	} else {
+		newQual := ComputeTicketQuality(ticket, senderPower)
+		// best ticket is lowest
+		if newQual < v.Quality {
+			v.Quality = newQual
+			c.values[key] = v
+		}
 	}
 	return nil
 }
 
-// FindMaxTicketProposal finds the value with the highest ticket, weighted by
-// sender power. Returns the self value (which may be zero) if and only if no
-// other value is found.
+// FindMaxTicketProposal finds the value with the best ticket, weighted by
+// sender power. Returns an invalid (zero-value) ConvergeValue if no converge is found.
 func (c *convergeState) FindMaxTicketProposal(table PowerTable) ConvergeValue {
 	// Non-determinism in case of matching tickets from an equivocation is ok.
 	// If the same ticket is used for two different values then either we get a decision on one of them
 	// only or we go to a new round. Eventually there is a round where the max ticket is held by a
 	// correct participant, who will not double vote.
-	var maxTicket *big.Int
-	var maxValue ConvergeValue
-
-	for key, value := range c.values {
-		for _, ticket := range c.tickets[key] {
-			senderPower, _ := table.Get(ticket.Sender)
-			ticketHash := blake2b.Sum256(ticket.Ticket)
-			ticketAsInt := new(big.Int).SetBytes(ticketHash[:])
-			weightedTicket := new(big.Int).Mul(ticketAsInt, big.NewInt(int64(senderPower)))
-			if maxTicket == nil || weightedTicket.Cmp(maxTicket) > 0 {
-				maxTicket = weightedTicket
-				maxValue = value
-			}
-		}
-	}
 
-	if maxTicket == nil && c.HasSelfValue() {
-		return *c.self
+	var bestValue ConvergeValue
+
+	for _, value := range c.values {
+		bestValue.TakeBetter(value)
 	}
-	return maxValue
+
+	return bestValue
 }
 
 // Finds some proposal which matches a specific value.
 // This searches values received in messages first, falling back to the participant's self value
 // only if necessary.
-func (c *convergeState) FindProposalFor(chain ECChain) (ConvergeValue, bool) {
+func (c *convergeState) FindProposalFor(chain ECChain) ConvergeValue {
 	for _, value := range c.values {
 		if value.Chain.Eq(chain) {
-			return value, true
+			return value
 		}
 	}
 
-	if c.HasSelfValue() && c.self.Chain.Eq(chain) {
-		return *c.self, true
-	}
-	return ConvergeValue{}, false
+	// Default converge value is not valid
+	return ConvergeValue{}
 }
 
 type broadcastState struct {

gpbft/ticket_quality.go

@@ -0,0 +1,69 @@
+package gpbft
+
+import (
+	"fmt"
+	"math"
+	"math/big"
+
+	"golang.org/x/crypto/blake2b"
+)
+
+// ComputeTicketQuality computes the quality of the ticket.
+// The lower the resulting quality the better.
+// We take the ticket, hash it using Blake2b256, take the low 128 bits, interpret them as a Q.128
+// fixed point number in range of [0, 1). Then we convert this uniform distribution into exponential one,
+// using -log(x) inverse distribution function.
+// The exponential distribution has a property where minimum of two exponentially distributed random
+// variables is itself a exponentially distributed.
+// This allows us to use the rate parameter to weight across different participants according to there power.
+// This ends up being `-log(ticket) / power` where ticket is [0, 1).
+// We additionally use log-base-2 instead of natural logarithm as it is easier to implement,
+// and it is just a linear factor on all tickets, meaning it does not influence their ordering.
+func ComputeTicketQuality(ticket []byte, power uint16) float64 {
+	// we could use Blake2b-128 but 256 is more common and more widely supported
+	ticketHash := blake2b.Sum256(ticket)
+	quality := linearToExpDist(ticketHash[:16])
+	return quality / float64(power)
+}
+
+// ticket should be 16 bytes
+func linearToExpDist(ticket []byte) float64 {
+	// we are interpreting the ticket as fixed-point number with 128 fractional bits
+	// and adjusting using exponential distribution inverse function, -log(x)
+	// we are computing Log2 of it with the adjustment that Log2(0) == -129
+	// we can use Log2 instead of Ln as the difference is linear transform between them which
+	// has no relative effect
+	asInt := new(big.Int).SetBytes(ticket) // interpret at Q.128
+	log2Int, log2Frac := bigLog2(asInt)
+	// combine integer and fractional parts, in theory we could operate on them separately
+	// but the 7bit gain on top of 52bits is minor
+	log2 := float64(log2Int) + log2Frac
+	return -log2
+}
+
+// bigLog2 takes an approximate logarithm of the big integer interpreted as Q.128
+// If the input is zero, the output is [-129, 0.f).
+// The result is an integer and fraction, where fraction is in [0, 1)
+func bigLog2(asInt *big.Int) (int64, float64) {
+	bitLen := uint(asInt.BitLen())
+	if bitLen == 0 {
+		return -129, 0.
+	}
+	log2Int := -int64(128 - bitLen + 1) //integer part of the Log2
+	// now that we saved the integer part, we want to interpret it as [1,2)
+	// so it will be Q.(bitlen-1)
+	// to convert to float exactly, we need to bring it down to 53 bits
+	if bitLen > 53 {
+		asInt = asInt.Rsh(asInt, bitLen-53)
+	} else if bitLen < 53 {
+		asInt = asInt.Lsh(asInt, 53-bitLen)
+	}
+	if asInt.BitLen() != 53 {
+		panic(fmt.Sprintf("wrong bitlen: %v", asInt.BitLen()))
+	}
+	asFloat := float64(asInt.Uint64()) / (1 << 52)
+	if asFloat < 1 || asFloat >= 2 {
+		panic("wrong range")
+	}
+	return log2Int, math.Log2(asFloat)
+}

gpbft/ticket_quality_test.go

@@ -0,0 +1,58 @@
+package gpbft
+
+import (
+	"bytes"
+	"math/big"
+	"runtime"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestTQ_BigLog2_Table(t *testing.T) {
+	tests := []struct {
+		name    string
+		input   string
+		integer int64
+		fract   float64
+	}{
+		{"0.(9)", "ffffffffffffffffffffffffffffffff", -1, 0.9999999999999999},
+		{"0.(9)8", "fffffffffffff8000000000000000000", -1, 0.9999999999999999},
+		{"0.(9)7", "fffffffffffff7000000000000000000", -1, 0.9999999999999997},
+		{"0.5", "80000000000000000000000000000000", -1, 0.0},
+		{"2^-128", "1", -128, 0.0},
+		{"2^-127", "2", -127, 0.0},
+		{"2^-127 + eps", "3", -127, 0.5849625007211563},
+		{"zero", "0", -129, 0.0},
+		{"medium", "10020000000000000", -64, 0.0007042690112466499},
+		{"medium2", "1000000000020000000000000", -32, 1.6409096303959814e-13},
+		{"2^(53-128)", "20000000000000", -75, 0.0},
+		{"2^(53-128)+eps", "20000000000001", -75, 0.0},
+		{"2^(53-128)-eps", "1fffffffffffff", -76, 0.9999999999999999},
+		{"2^(53-128)-2eps", "1ffffffffffff3", -76, 0.9999999999999979},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			bigInt, ok := new(big.Int).SetString(test.input, 16)
+			require.True(t, ok, "parsing int")
+			integer, fract := bigLog2(bigInt)
+			assert.EqualValues(t, test.integer, integer, "wrong integer part")
+			assert.EqualValues(t, test.fract, fract, "wrong fractional part")
+		})
+	}
+}
+
+func FuzzTQ_linearToExp(f *testing.F) {
+	f.Add(make([]byte, 16))
+	f.Add(bytes.Repeat([]byte{0xff}, 16))
+	f.Add(bytes.Repeat([]byte{0xa0}, 16))
+	f.Fuzz(func(t *testing.T, ticket []byte) {
+		if len(ticket) != 16 {
+			return
+		}
+		q := linearToExpDist(ticket)
+		runtime.KeepAlive(q)
+	})
+}