Skip to content

Commit

Permalink
fix(store): correctly break weight ties based on smaller ticket (#12253)
Browse files Browse the repository at this point in the history
@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`.
  • Loading branch information
arajasek authored Jul 19, 2024
1 parent 11dd4c9 commit 124d0a4
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 2 deletions.
7 changes: 5 additions & 2 deletions chain/store/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
75 changes: 75 additions & 0 deletions chain/store/weight_test.go
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit 124d0a4

Please sign in to comment.