fix(store): correctly break weight ties based on smaller ticket (file…

…coin-project#12253)

@rjan90 and @jennijuju reported seeing this log a lot, which prompted an investigation of the logic. This code does not implement [FIP-0023](https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0023.md) correctly. Specifically, if we have the following mintickets:

```
ts1: 1, 2
ts2: 0,3
```

This code will incorrectly return ts1 as the "smaller" tipset, even though the tie should have been broken in favour of ts2 based on `1 > 0`.

chain/store/store.go

@@ -1322,12 +1322,15 @@ func breakWeightTie(ts1, ts2 *types.TipSet) bool {
 	// blocks are already sorted by ticket
 	for i := 0; i < s; i++ {
 		if ts1.Blocks()[i].Ticket.Less(ts2.Blocks()[i].Ticket) {
-			log.Infof("weight tie broken in favour of %s", ts1.Key())
+			log.Infof("weight tie broken in favour of %s against %s", ts1.Key(), ts2.Key())
 			return true
+		} else if ts2.Blocks()[i].Ticket.Less(ts1.Blocks()[i].Ticket) {
+			log.Infof("weight tie broken in favour of %s against %s", ts2.Key(), ts1.Key())
+			return false
 		}
 	}
 
-	log.Infof("weight tie left unbroken, default to %s", ts2.Key())
+	log.Warnf("weight tie between %s and %s left unbroken, default to %s", ts1.Key(), ts2.Key(), ts2.Key())
 	return false
 }
 

chain/store/weight_test.go

@@ -0,0 +1,75 @@
+package store
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/ipfs/go-cid"
+	"github.com/minio/blake2b-simd"
+	"github.com/stretchr/testify/require"
+
+	"github.com/filecoin-project/go-address"
+	"github.com/filecoin-project/go-state-types/abi"
+
+	"github.com/filecoin-project/lotus/chain/types"
+)
+
+func TestWeightTieBreaker(t *testing.T) {
+
+	smallerVrf1 := []byte{0} // blake2b starts with byte 3
+	largerVrf1 := []byte{2}  // blake2b starts with byte 187
+
+	smallerVrfSum1 := blake2b.Sum256(smallerVrf1)
+	largerVrfSum1 := blake2b.Sum256(largerVrf1)
+
+	// this checks that smallerVrf is in fact smaller than LargerVrf
+	require.True(t, bytes.Compare(smallerVrfSum1[:], largerVrfSum1[:]) < 0)
+
+	smallerVrf2 := []byte{3} // blake2b starts with byte 232
+	largerVrf2 := []byte{1}  // blake2b starts with byte 238
+
+	smallerVrfSum2 := blake2b.Sum256(smallerVrf2)
+	largerVrfSum2 := blake2b.Sum256(largerVrf2)
+
+	// this checks that smallerVrf is in fact smaller than LargerVrf
+	require.True(t, bytes.Compare(smallerVrfSum2[:], largerVrfSum2[:]) < 0)
+
+	ts1 := mockTipSet(t)
+	ts2 := mockTipSet(t)
+
+	ts1.Blocks()[0].Ticket = &types.Ticket{VRFProof: largerVrf1}
+	ts2.Blocks()[0].Ticket = &types.Ticket{VRFProof: smallerVrf1}
+	ts1.Blocks()[1].Ticket = &types.Ticket{VRFProof: smallerVrf2}
+	ts2.Blocks()[1].Ticket = &types.Ticket{VRFProof: largerVrf2}
+
+	// ts1's first block has a larger VRF than ts2's, so it should lose
+	require.False(t, breakWeightTie(ts1, ts2))
+}
+
+func mockTipSet(t *testing.T) *types.TipSet {
+	minerAct, err := address.NewIDAddress(0)
+	require.NoError(t, err)
+	c, err := cid.Decode("QmbFMke1KXqnYyBBWxB74N4c5SBnJMVAiMNRcGu6x1AwQH")
+	require.NoError(t, err)
+	blks := []*types.BlockHeader{
+		{
+			Miner:                 minerAct,
+			Height:                abi.ChainEpoch(1),
+			ParentStateRoot:       c,
+			ParentMessageReceipts: c,
+			Messages:              c,
+			Ticket:                &types.Ticket{VRFProof: []byte{}},
+		},
+		{
+			Miner:                 minerAct,
+			Height:                abi.ChainEpoch(1),
+			ParentStateRoot:       c,
+			ParentMessageReceipts: c,
+			Messages:              c,
+			Ticket:                &types.Ticket{VRFProof: []byte{}},
+		},
+	}
+	ts, err := types.NewTipSet(blks)
+	require.NoError(t, err)
+	return ts
+}