Fix BestFinalized method (#7619)

* turn tests to use testing vector

* add regression test case

* complete tests
This commit is contained in:
Victor Farazdagi 2020-10-23 05:49:42 +03:00 committed by GitHub
parent ecc25d2b8c
commit 6a2bb65fe2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 143 additions and 56 deletions

View File

@ -426,11 +426,12 @@ func (p *Status) Prune() {
}
}
// BestFinalized returns the highest finalized epoch equal to or higher than ours that is agreed upon by the majority of peers.
// This method may not return the absolute highest finalized, but the finalized epoch in which most peers can serve blocks.
// Ideally, all peers would be reporting the same finalized epoch but some may be behind due to their own latency, or because of
// their finalized epoch at the time we queried them.
// Returns the best finalized root, epoch number, and list of peers that are at or beyond that epoch.
// BestFinalized returns the highest finalized epoch equal to or higher than ours that is agreed
// upon by the majority of peers. This method may not return the absolute highest finalized, but
// the finalized epoch in which most peers can serve blocks (plurality voting).
// Ideally, all peers would be reporting the same finalized epoch but some may be behind due to their
// own latency, or because of their finalized epoch at the time we queried them.
// Returns epoch number and list of peers that are at or beyond that epoch.
func (p *Status) BestFinalized(maxPeers int, ourFinalizedEpoch uint64) (uint64, []peer.ID) {
connected := p.Connected()
finalizedEpochVotes := make(map[uint64]uint64)
@ -459,8 +460,10 @@ func (p *Status) BestFinalized(maxPeers int, ourFinalizedEpoch uint64) (uint64,
// Sort PIDs by finalized epoch, in decreasing order.
sort.Slice(potentialPIDs, func(i, j int) bool {
return pidEpoch[potentialPIDs[i]] > pidEpoch[potentialPIDs[j]] &&
pidHead[potentialPIDs[i]] > pidHead[potentialPIDs[j]]
if pidEpoch[potentialPIDs[i]] == pidEpoch[potentialPIDs[j]] {
return pidHead[potentialPIDs[i]] > pidHead[potentialPIDs[j]]
}
return pidEpoch[potentialPIDs[i]] > pidEpoch[potentialPIDs[j]]
})
// Trim potential peers to those on or after target epoch.

View File

@ -536,58 +536,142 @@ func TestTrimmedOrderedPeers(t *testing.T) {
assert.Equal(t, pid1, pids[2], "Incorrect third peer")
}
func TestBestPeer(t *testing.T) {
maxBadResponses := 2
expectedFinEpoch := uint64(4)
expectedRoot := [32]byte{'t', 'e', 's', 't'}
junkRoot := [32]byte{'j', 'u', 'n', 'k'}
p := peers.NewStatus(context.Background(), &peers.StatusConfig{
PeerLimit: 30,
ScorerParams: &scorers.Config{
BadResponsesScorerConfig: &scorers.BadResponsesScorerConfig{
Threshold: maxBadResponses,
func TestStatus_BestPeer(t *testing.T) {
type peerConfig struct {
headSlot uint64
finalizedEpoch uint64
}
tests := []struct {
name string
peers []*peerConfig
limitPeers int
ourFinalizedEpoch uint64
targetEpoch uint64
// targetEpochSupport denotes how many peers support returned epoch.
targetEpochSupport int
}{
{
name: "head slot matches finalized epoch",
peers: []*peerConfig{
{finalizedEpoch: 4, headSlot: 4 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 4, headSlot: 4 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 3, headSlot: 3 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 4, headSlot: 4 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 4, headSlot: 4 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 3, headSlot: 3 * params.BeaconConfig().SlotsPerEpoch},
},
limitPeers: 15,
targetEpoch: 4,
targetEpochSupport: 4,
},
})
{
// Peers are compared using their finalized epoch, head should not affect peer selection.
// Test case below is a regression case: to ensure that only epoch is used indeed.
// (Function sorts peers, and on equal head slot, produced incorrect results).
name: "head slots equal for peers with different finalized epochs",
peers: []*peerConfig{
{finalizedEpoch: 4, headSlot: 4 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 4, headSlot: 4 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 3, headSlot: 4 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 4, headSlot: 4 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 4, headSlot: 4 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 3, headSlot: 4 * params.BeaconConfig().SlotsPerEpoch},
},
limitPeers: 15,
targetEpoch: 4,
targetEpochSupport: 4,
},
{
name: "head slot significantly ahead of finalized epoch (long period of non-finality)",
peers: []*peerConfig{
{finalizedEpoch: 4, headSlot: 42 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 4, headSlot: 42 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 3, headSlot: 42 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 4, headSlot: 42 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 4, headSlot: 42 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 3, headSlot: 42 * params.BeaconConfig().SlotsPerEpoch},
},
limitPeers: 15,
targetEpoch: 4,
targetEpochSupport: 4,
},
{
name: "ignore lower epoch peers",
peers: []*peerConfig{
{finalizedEpoch: 4, headSlot: 41 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 4, headSlot: 42 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 3, headSlot: 43 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 4, headSlot: 44 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 4, headSlot: 45 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 3, headSlot: 46 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 6, headSlot: 6 * params.BeaconConfig().SlotsPerEpoch},
},
ourFinalizedEpoch: 5,
limitPeers: 15,
targetEpoch: 6,
targetEpochSupport: 1,
},
{
name: "combine peers from several epochs starting from epoch higher than ours",
peers: []*peerConfig{
{finalizedEpoch: 4, headSlot: 41 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 4, headSlot: 42 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 3, headSlot: 43 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 4, headSlot: 44 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 4, headSlot: 45 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 3, headSlot: 46 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 6, headSlot: 6 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 6, headSlot: 6 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 6, headSlot: 6 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 7, headSlot: 7 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 8, headSlot: 8 * params.BeaconConfig().SlotsPerEpoch},
},
ourFinalizedEpoch: 5,
limitPeers: 15,
targetEpoch: 6,
targetEpochSupport: 5,
},
{
name: "limit number of returned peers",
peers: []*peerConfig{
{finalizedEpoch: 4, headSlot: 41 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 4, headSlot: 42 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 3, headSlot: 43 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 4, headSlot: 44 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 4, headSlot: 45 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 3, headSlot: 46 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 6, headSlot: 6 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 6, headSlot: 6 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 6, headSlot: 6 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 7, headSlot: 7 * params.BeaconConfig().SlotsPerEpoch},
{finalizedEpoch: 8, headSlot: 8 * params.BeaconConfig().SlotsPerEpoch},
},
ourFinalizedEpoch: 5,
limitPeers: 4,
targetEpoch: 6,
targetEpochSupport: 4,
},
}
// Peer 1
pid1 := addPeer(t, p, peers.PeerConnected)
p.SetChainState(pid1, &pb.Status{
FinalizedEpoch: expectedFinEpoch,
FinalizedRoot: expectedRoot[:],
})
// Peer 2
pid2 := addPeer(t, p, peers.PeerConnected)
p.SetChainState(pid2, &pb.Status{
FinalizedEpoch: expectedFinEpoch,
FinalizedRoot: expectedRoot[:],
})
// Peer 3
pid3 := addPeer(t, p, peers.PeerConnected)
p.SetChainState(pid3, &pb.Status{
FinalizedEpoch: 3,
FinalizedRoot: junkRoot[:],
})
// Peer 4
pid4 := addPeer(t, p, peers.PeerConnected)
p.SetChainState(pid4, &pb.Status{
FinalizedEpoch: expectedFinEpoch,
FinalizedRoot: expectedRoot[:],
})
// Peer 5
pid5 := addPeer(t, p, peers.PeerConnected)
p.SetChainState(pid5, &pb.Status{
FinalizedEpoch: expectedFinEpoch,
FinalizedRoot: expectedRoot[:],
})
// Peer 6
pid6 := addPeer(t, p, peers.PeerConnected)
p.SetChainState(pid6, &pb.Status{
FinalizedEpoch: 3,
FinalizedRoot: junkRoot[:],
})
retEpoch, _ := p.BestFinalized(15, 0)
assert.Equal(t, expectedFinEpoch, retEpoch, "Incorrect Finalized epoch retrieved")
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
p := peers.NewStatus(context.Background(), &peers.StatusConfig{
PeerLimit: 30,
ScorerParams: &scorers.Config{
BadResponsesScorerConfig: &scorers.BadResponsesScorerConfig{Threshold: 2},
},
})
for _, peerConfig := range tt.peers {
p.SetChainState(addPeer(t, p, peers.PeerConnected), &pb.Status{
FinalizedEpoch: peerConfig.finalizedEpoch,
HeadSlot: peerConfig.headSlot,
})
}
epoch, pids := p.BestFinalized(tt.limitPeers, tt.ourFinalizedEpoch)
assert.Equal(t, tt.targetEpoch, epoch, "Unexpected epoch retrieved")
assert.Equal(t, tt.targetEpochSupport, len(pids), "Unexpected number of peers supporting retrieved epoch")
})
}
}
func TestBestFinalized_returnsMaxValue(t *testing.T) {