Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove equivocating votes from forkchoice #10597

Merged
merged 5 commits into from
May 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions beacon-chain/forkchoice/doubly-linked-tree/forkchoice.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func New(justifiedEpoch, finalizedEpoch types.Epoch) *ForkChoice {
proposerBoostRoot: [32]byte{},
nodeByRoot: make(map[[fieldparams.RootLength]byte]*Node),
nodeByPayload: make(map[[fieldparams.RootLength]byte]*Node),
slashedIndices: make(map[types.ValidatorIndex]bool),
pruneThreshold: defaultPruneThreshold,
}

Expand Down Expand Up @@ -216,6 +217,10 @@ func (f *ForkChoice) AncestorRoot(ctx context.Context, root [32]byte, slot types
// validators' latest votes.
func (f *ForkChoice) updateBalances(newBalances []uint64) error {
for index, vote := range f.votes {
// Skip if validator has been slashed
if f.store.slashedIndices[types.ValidatorIndex(index)] {
continue
}
// Skip if validator has never voted for current root and next root (i.e. if the
// votes are zero hash aka genesis block), there's nothing to compute.
if vote.currentRoot == params.BeaconConfig().ZeroHash && vote.nextRoot == params.BeaconConfig().ZeroHash {
Expand Down Expand Up @@ -321,3 +326,35 @@ func (f *ForkChoice) ForkChoiceNodes() []*pbrpc.ForkChoiceNode {
func (f *ForkChoice) SetOptimisticToInvalid(ctx context.Context, root, parentRoot, payloadHash [fieldparams.RootLength]byte) ([][32]byte, error) {
return f.store.setOptimisticToInvalid(ctx, root, parentRoot, payloadHash)
}

// InsertSlashedIndex adds the given slashed validator index to the
// store-tracked list. Votes from these validators are not accounted for
// in forkchoice.
func (f *ForkChoice) InsertSlashedIndex(_ context.Context, index types.ValidatorIndex) {
f.store.nodesLock.Lock()
defer f.store.nodesLock.Unlock()
f.store.slashedIndices[index] = true

// Subtract last vote from this equivocating validator
f.votesLock.RLock()
defer f.votesLock.RUnlock()

if index > types.ValidatorIndex(len(f.balances)) {
return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if these plain returns should really be error returns

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need for an error, these could be for example a new validator who's first transaction is a slashable offense. In this case we would honestly not have these balances in forkchoice.

}

if index > types.ValidatorIndex(len(f.votes)) {
return
}

node, ok := f.store.nodeByRoot[f.votes[index].currentRoot]
if !ok || node == nil {
return
}

if node.balance < f.balances[index] {
node.balance = 0
} else {
node.balance -= f.balances[index]
}
}
33 changes: 33 additions & 0 deletions beacon-chain/forkchoice/doubly-linked-tree/forkchoice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,39 @@ func TestForkChoice_AncestorLowerSlot(t *testing.T) {
require.Equal(t, root, [32]byte{'1'})
}

func TestForkChoice_RemoveEquivocating(t *testing.T) {
ctx := context.Background()
f := setup(1, 1)
// Insert a block it will be head
require.NoError(t, f.InsertOptimisticBlock(ctx, 1, [32]byte{'a'}, params.BeaconConfig().ZeroHash, [32]byte{'A'}, 1, 1))
head, err := f.Head(ctx, 1, params.BeaconConfig().ZeroHash, []uint64{}, 1)
require.NoError(t, err)
require.Equal(t, [32]byte{'a'}, head)

// Insert two extra blocks
require.NoError(t, f.InsertOptimisticBlock(ctx, 2, [32]byte{'b'}, [32]byte{'a'}, [32]byte{'B'}, 1, 1))
require.NoError(t, f.InsertOptimisticBlock(ctx, 3, [32]byte{'c'}, [32]byte{'a'}, [32]byte{'C'}, 1, 1))
head, err = f.Head(ctx, 1, params.BeaconConfig().ZeroHash, []uint64{}, 1)
require.NoError(t, err)
require.Equal(t, [32]byte{'c'}, head)

// Insert two attestations for block b, one for c it becomes head
f.ProcessAttestation(ctx, []uint64{1, 2}, [32]byte{'b'}, 1)
f.ProcessAttestation(ctx, []uint64{3}, [32]byte{'c'}, 1)
head, err = f.Head(ctx, 1, params.BeaconConfig().ZeroHash, []uint64{100, 200, 200, 300}, 1)
require.NoError(t, err)
require.Equal(t, [32]byte{'b'}, head)

// Process b's slashing, c is now head
f.InsertSlashedIndex(ctx, 1)
require.Equal(t, uint64(200), f.store.nodeByRoot[[32]byte{'b'}].balance)
head, err = f.Head(ctx, 1, params.BeaconConfig().ZeroHash, []uint64{100, 200, 200, 300}, 1)
require.Equal(t, uint64(200), f.store.nodeByRoot[[32]byte{'b'}].weight)
require.Equal(t, uint64(300), f.store.nodeByRoot[[32]byte{'c'}].weight)
require.NoError(t, err)
require.Equal(t, [32]byte{'c'}, head)
}

func indexToHash(i uint64) [32]byte {
var b [8]byte
binary.LittleEndian.PutUint64(b[:], i)
Expand Down
1 change: 1 addition & 0 deletions beacon-chain/forkchoice/doubly-linked-tree/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ type Store struct {
headNode *Node // last head Node
nodeByRoot map[[fieldparams.RootLength]byte]*Node // nodes indexed by roots.
nodeByPayload map[[fieldparams.RootLength]byte]*Node // nodes indexed by payload Hash
slashedIndices map[types.ValidatorIndex]bool // the list of equivocating validator indices
nodesLock sync.RWMutex
proposerBoostLock sync.RWMutex
}
Expand Down
1 change: 1 addition & 0 deletions beacon-chain/forkchoice/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ type BlockProcessor interface {
// AttestationProcessor processes the attestation that's used for accounting fork choice.
type AttestationProcessor interface {
ProcessAttestation(context.Context, []uint64, [32]byte, types.Epoch)
InsertSlashedIndex(context.Context, types.ValidatorIndex)
}

// Pruner prunes the fork choice upon new finalization. This is used to keep fork choice sane.
Expand Down
6 changes: 6 additions & 0 deletions beacon-chain/forkchoice/protoarray/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"

"github.com/prysmaticlabs/prysm/config/params"
types "github.com/prysmaticlabs/prysm/consensus-types/primitives"
pmath "github.com/prysmaticlabs/prysm/math"
"go.opencensus.io/trace"
)
Expand All @@ -15,13 +16,18 @@ func computeDeltas(
blockIndices map[[32]byte]uint64,
votes []Vote,
oldBalances, newBalances []uint64,
slashedIndices map[types.ValidatorIndex]bool,
) ([]int, []Vote, error) {
_, span := trace.StartSpan(ctx, "doublyLinkedForkchoice.computeDeltas")
defer span.End()

deltas := make([]int, len(blockIndices))

for validatorIndex, vote := range votes {
// Skip if validator has been slashed
if slashedIndices[types.ValidatorIndex(validatorIndex)] {
continue
}
oldBalance := uint64(0)
newBalance := uint64(0)

Expand Down
25 changes: 17 additions & 8 deletions beacon-chain/forkchoice/protoarray/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"testing"

"github.com/prysmaticlabs/prysm/config/params"
types "github.com/prysmaticlabs/prysm/consensus-types/primitives"
"github.com/prysmaticlabs/prysm/crypto/hash"
"github.com/prysmaticlabs/prysm/testing/assert"
"github.com/prysmaticlabs/prysm/testing/require"
Expand All @@ -25,7 +26,8 @@ func TestComputeDelta_ZeroHash(t *testing.T) {
newBalances = append(newBalances, 0)
}

delta, _, err := computeDeltas(context.Background(), indices, votes, oldBalances, newBalances)
slashedIndices := make(map[types.ValidatorIndex]bool)
delta, _, err := computeDeltas(context.Background(), indices, votes, oldBalances, newBalances, slashedIndices)
require.NoError(t, err)
assert.Equal(t, int(validatorCount), len(delta))

Expand All @@ -52,7 +54,8 @@ func TestComputeDelta_AllVoteTheSame(t *testing.T) {
newBalances = append(newBalances, balance)
}

delta, _, err := computeDeltas(context.Background(), indices, votes, oldBalances, newBalances)
slashedIndices := make(map[types.ValidatorIndex]bool)
delta, _, err := computeDeltas(context.Background(), indices, votes, oldBalances, newBalances, slashedIndices)
require.NoError(t, err)
assert.Equal(t, int(validatorCount), len(delta))

Expand Down Expand Up @@ -84,7 +87,8 @@ func TestComputeDelta_DifferentVotes(t *testing.T) {
newBalances = append(newBalances, balance)
}

delta, _, err := computeDeltas(context.Background(), indices, votes, oldBalances, newBalances)
slashedIndices := make(map[types.ValidatorIndex]bool)
delta, _, err := computeDeltas(context.Background(), indices, votes, oldBalances, newBalances, slashedIndices)
require.NoError(t, err)
assert.Equal(t, int(validatorCount), len(delta))

Expand Down Expand Up @@ -113,7 +117,8 @@ func TestComputeDelta_MovingVotes(t *testing.T) {
newBalances = append(newBalances, balance)
}

delta, _, err := computeDeltas(context.Background(), indices, votes, oldBalances, newBalances)
slashedIndices := make(map[types.ValidatorIndex]bool)
delta, _, err := computeDeltas(context.Background(), indices, votes, oldBalances, newBalances, slashedIndices)
require.NoError(t, err)
assert.Equal(t, int(validatorCount), len(delta))

Expand Down Expand Up @@ -145,7 +150,8 @@ func TestComputeDelta_MoveOutOfTree(t *testing.T) {
Vote{indexToHash(1), params.BeaconConfig().ZeroHash, 0},
Vote{indexToHash(1), [32]byte{'A'}, 0})

delta, _, err := computeDeltas(context.Background(), indices, votes, oldBalances, newBalances)
slashedIndices := make(map[types.ValidatorIndex]bool)
delta, _, err := computeDeltas(context.Background(), indices, votes, oldBalances, newBalances, slashedIndices)
require.NoError(t, err)
assert.Equal(t, 1, len(delta))
assert.Equal(t, 0-2*int(balance), delta[0])
Expand Down Expand Up @@ -173,7 +179,8 @@ func TestComputeDelta_ChangingBalances(t *testing.T) {
newBalances = append(newBalances, newBalance)
}

delta, _, err := computeDeltas(context.Background(), indices, votes, oldBalances, newBalances)
slashedIndices := make(map[types.ValidatorIndex]bool)
delta, _, err := computeDeltas(context.Background(), indices, votes, oldBalances, newBalances, slashedIndices)
require.NoError(t, err)
assert.Equal(t, 16, len(delta))

Expand Down Expand Up @@ -206,7 +213,8 @@ func TestComputeDelta_ValidatorAppear(t *testing.T) {
Vote{indexToHash(1), indexToHash(2), 0},
Vote{indexToHash(1), indexToHash(2), 0})

delta, _, err := computeDeltas(context.Background(), indices, votes, oldBalances, newBalances)
slashedIndices := make(map[types.ValidatorIndex]bool)
delta, _, err := computeDeltas(context.Background(), indices, votes, oldBalances, newBalances, slashedIndices)
require.NoError(t, err)
assert.Equal(t, 2, len(delta))
assert.Equal(t, 0-int(balance), delta[0])
Expand All @@ -231,7 +239,8 @@ func TestComputeDelta_ValidatorDisappears(t *testing.T) {
Vote{indexToHash(1), indexToHash(2), 0},
Vote{indexToHash(1), indexToHash(2), 0})

delta, _, err := computeDeltas(context.Background(), indices, votes, oldBalances, newBalances)
slashedIndices := make(map[types.ValidatorIndex]bool)
delta, _, err := computeDeltas(context.Background(), indices, votes, oldBalances, newBalances, slashedIndices)
require.NoError(t, err)
assert.Equal(t, 2, len(delta))
assert.Equal(t, 0-2*int(balance), delta[0])
Expand Down
48 changes: 47 additions & 1 deletion beacon-chain/forkchoice/protoarray/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ func New(justifiedEpoch, finalizedEpoch types.Epoch, finalizedRoot [32]byte) *Fo
nodesIndices: make(map[[32]byte]uint64),
payloadIndices: make(map[[32]byte]uint64),
canonicalNodes: make(map[[32]byte]bool),
slashedIndices: make(map[types.ValidatorIndex]bool),
pruneThreshold: defaultPruneThreshold,
}

Expand Down Expand Up @@ -63,7 +64,7 @@ func (f *ForkChoice) Head(
// Using the write lock here because `updateCanonicalNodes` that gets called subsequently requires a write operation.
f.store.nodesLock.Lock()
defer f.store.nodesLock.Unlock()
deltas, newVotes, err := computeDeltas(ctx, f.store.nodesIndices, f.votes, f.balances, newBalances)
deltas, newVotes, err := computeDeltas(ctx, f.store.nodesIndices, f.votes, f.balances, newBalances, f.store.slashedIndices)
if err != nil {
return [32]byte{}, errors.Wrap(err, "Could not compute deltas")
}
Expand Down Expand Up @@ -741,3 +742,48 @@ func (f *ForkChoice) ForkChoiceNodes() []*pbrpc.ForkChoiceNode {
}
return ret
}

// InsertSlashedIndex adds the given slashed validator index to the
// store-tracked list. Votes from these validators are not accounted for
// in forkchoice.
func (f *ForkChoice) InsertSlashedIndex(ctx context.Context, index types.ValidatorIndex) {
f.store.nodesLock.Lock()
defer f.store.nodesLock.Unlock()
f.store.slashedIndices[index] = true

// Subtract last vote from this equivocating validator
f.votesLock.RLock()
defer f.votesLock.RUnlock()

if index > types.ValidatorIndex(len(f.balances)) {
return
}

if index > types.ValidatorIndex(len(f.votes)) {
return
}

nodeIndex, ok := f.store.nodesIndices[f.votes[index].currentRoot]
if !ok {
return
}

var node *Node
for nodeIndex != NonExistentNode {
if ctx.Err() != nil {
return
}

node = f.store.nodes[nodeIndex]
if node == nil {
return
}

if node.weight < f.balances[index] {
node.weight = 0
} else {
node.weight -= f.balances[index]
}
nodeIndex = node.parent
}
}
29 changes: 29 additions & 0 deletions beacon-chain/forkchoice/protoarray/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -773,3 +773,32 @@ func TestStore_UpdateCanonicalNodes_RemoveOldCanonical(t *testing.T) {
_, ok := f.store.canonicalNodes[[32]byte{'c'}]
require.Equal(t, false, ok)
}

func TestStore_RemoveEquivocating(t *testing.T) {
ctx := context.Background()
f := setup(1, 1)
// Insert a block it will be head
require.NoError(t, f.InsertOptimisticBlock(ctx, 1, [32]byte{'a'}, params.BeaconConfig().ZeroHash, [32]byte{'A'}, 1, 1))
head, err := f.Head(ctx, 1, params.BeaconConfig().ZeroHash, []uint64{}, 1)
require.NoError(t, err)
require.Equal(t, [32]byte{'a'}, head)

// Insert two extra blocks
require.NoError(t, f.InsertOptimisticBlock(ctx, 2, [32]byte{'b'}, [32]byte{'a'}, [32]byte{'B'}, 1, 1))
require.NoError(t, f.InsertOptimisticBlock(ctx, 3, [32]byte{'c'}, [32]byte{'a'}, [32]byte{'C'}, 1, 1))
head, err = f.Head(ctx, 1, params.BeaconConfig().ZeroHash, []uint64{}, 1)
require.NoError(t, err)
require.Equal(t, [32]byte{'c'}, head)

// Insert an attestation for block b, it becomes head
f.ProcessAttestation(ctx, []uint64{1}, [32]byte{'b'}, 1)
head, err = f.Head(ctx, 1, params.BeaconConfig().ZeroHash, []uint64{100, 200}, 1)
require.NoError(t, err)
require.Equal(t, [32]byte{'b'}, head)

// Process b's slashing, c is now head
f.InsertSlashedIndex(ctx, 1)
head, err = f.Head(ctx, 1, params.BeaconConfig().ZeroHash, []uint64{100, 200}, 1)
require.NoError(t, err)
require.Equal(t, [32]byte{'c'}, head)
}
1 change: 1 addition & 0 deletions beacon-chain/forkchoice/protoarray/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type Store struct {
nodesIndices map[[fieldparams.RootLength]byte]uint64 // the root of block node and the nodes index in the list.
canonicalNodes map[[fieldparams.RootLength]byte]bool // the canonical block nodes.
payloadIndices map[[fieldparams.RootLength]byte]uint64 // the payload hash of block node and the index in the list
slashedIndices map[types.ValidatorIndex]bool // The list of equivocating validators
nodesLock sync.RWMutex
proposerBoostLock sync.RWMutex
}
Expand Down