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.

Sign up for GitHub

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

Accept attestations when node is optimistic #11319

Merged
merged 5 commits into from
Aug 25, 2022
+4 −88
Merged

Accept attestations when node is optimistic #11319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
416dabf
Accept attestations when node is optimistic
terencechain Aug 25, 2022
e4bcc5e
Fix tests
terencechain Aug 25, 2022
612e896
Add regression tests
terencechain Aug 25, 2022
02e7c0c
Fix tests
terencechain Aug 25, 2022
078abac
Fix more bad tests
terencechain Aug 25, 2022
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
10 changes: 0 additions & 10 deletions beacon-chain/sync/validate_aggregate_proof.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,6 @@ func (s *Service) validateAggregateAndProof(ctx context.Context, pid peer.ID, ms
return pubsub.ValidationIgnore, nil
}

// We should not attempt to process this message if the node is running in optimistic mode.
// We just ignore in p2p so that the peer is not penalized.
optimistic, err := s.cfg.chain.IsOptimistic(ctx)
if err != nil {
return pubsub.ValidationReject, err
}
if optimistic {
return pubsub.ValidationIgnore, nil
}

raw, err := s.decodePubsubMessage(msg)
if err != nil {
tracing.AnnotateError(span, err)
Expand Down
33 changes: 1 addition & 32 deletions beacon-chain/sync/validate_aggregate_proof_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,7 @@ func TestValidateAggregateAndProof_CanValidate(t *testing.T) {
beaconDB: db,
initialSync: &mockSync.Sync{IsSyncing: false},
chain: &mock.ChainService{Genesis: time.Now().Add(-oneEpoch()),
Optimistic: true,
DB: db,
State: beaconState,
ValidAttestation: true,
Expand Down Expand Up @@ -697,35 +698,3 @@ func TestValidateAggregateAndProof_RejectWhenAttEpochDoesntEqualTargetEpoch(t *t
assert.NotNil(t, err)
assert.Equal(t, pubsub.ValidationReject, res)
}

func TestValidateAggregateAndProof_Optimistic(t *testing.T) {
p := p2ptest.NewTestP2P(t)
ctx := context.Background()

exit, s := setupValidExit(t)

r := &Service{
cfg: &config{
p2p: p,
chain: &mock.ChainService{
State: s,
Optimistic: true,
},
initialSync: &mockSync.Sync{IsSyncing: false},
},
}
buf := new(bytes.Buffer)
_, err := p.Encoding().EncodeGossip(buf, exit)
require.NoError(t, err)
topic := p2p.GossipTypeMapping[reflect.TypeOf(exit)]
m := &pubsub.Message{
Message: &pubsubpb.Message{
Data: buf.Bytes(),
Topic: &topic,
},
}
res, err := r.validateAggregateAndProof(ctx, "", m)
assert.NoError(t, err)
valid := res == pubsub.ValidationIgnore
assert.Equal(t, true, valid, "Validation should have ignored the message")
}
10 changes: 0 additions & 10 deletions beacon-chain/sync/validate_beacon_attestation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,6 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(ctx context.Context, p
return pubsub.ValidationIgnore, nil
}

// We should not attempt to process this message if the node is running in optimistic mode.
// We just ignore in p2p so that the peer is not penalized.
optimistic, err := s.cfg.chain.IsOptimistic(ctx)
if err != nil {
return pubsub.ValidationReject, err
}
if optimistic {
return pubsub.ValidationIgnore, nil
}

ctx, span := trace.StartSpan(ctx, "sync.validateCommitteeIndexBeaconAttestation")
defer span.End()

Expand Down
35 changes: 1 addition & 34 deletions beacon-chain/sync/validate_beacon_attestation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"context"
"fmt"
"reflect"
"testing"
"time"

Expand All @@ -15,15 +14,13 @@ import (
"github.com/prysmaticlabs/prysm/v3/beacon-chain/core/helpers"
"github.com/prysmaticlabs/prysm/v3/beacon-chain/core/signing"
dbtest "github.com/prysmaticlabs/prysm/v3/beacon-chain/db/testing"
"github.com/prysmaticlabs/prysm/v3/beacon-chain/p2p"
p2ptest "github.com/prysmaticlabs/prysm/v3/beacon-chain/p2p/testing"
mockSync "github.com/prysmaticlabs/prysm/v3/beacon-chain/sync/initial-sync/testing"
lruwrpr "github.com/prysmaticlabs/prysm/v3/cache/lru"
fieldparams "github.com/prysmaticlabs/prysm/v3/config/fieldparams"
"github.com/prysmaticlabs/prysm/v3/config/params"
"github.com/prysmaticlabs/prysm/v3/encoding/bytesutil"
ethpb "github.com/prysmaticlabs/prysm/v3/proto/prysm/v1alpha1"
"github.com/prysmaticlabs/prysm/v3/testing/assert"
"github.com/prysmaticlabs/prysm/v3/testing/require"
"github.com/prysmaticlabs/prysm/v3/testing/util"
)
Expand All @@ -38,6 +35,7 @@ func TestService_validateCommitteeIndexBeaconAttestation(t *testing.T) {
ValidatorsRoot: [32]byte{'A'},
ValidAttestation: true,
DB: db,
Optimistic: true,
}
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
Expand Down Expand Up @@ -306,37 +304,6 @@ func TestService_validateCommitteeIndexBeaconAttestation(t *testing.T) {
}
}

func TestServiceValidateCommitteeIndexBeaconAttestation_Optimistic(t *testing.T) {
p := p2ptest.NewTestP2P(t)
ctx := context.Background()

slashing, s := setupValidAttesterSlashing(t)

r := &Service{
cfg: &config{
p2p: p,
chain: &mockChain.ChainService{State: s, Optimistic: true},
initialSync: &mockSync.Sync{IsSyncing: false},
},
}

buf := new(bytes.Buffer)
_, err := p.Encoding().EncodeGossip(buf, slashing)
require.NoError(t, err)

topic := p2p.GossipTypeMapping[reflect.TypeOf(slashing)]
msg := &pubsub.Message{
Message: &pubsubpb.Message{
Data: buf.Bytes(),
Topic: &topic,
},
}
res, err := r.validateCommitteeIndexBeaconAttestation(ctx, "foobar", msg)
assert.NoError(t, err)
valid := res == pubsub.ValidationIgnore
assert.Equal(t, true, valid, "Should have ignore this message")
}

func TestService_setSeenCommitteeIndicesSlot(t *testing.T) {
chainService := &mockChain.ChainService{
Genesis: time.Now(),
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/sync/validate_sync_committee_message_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ func TestValidateSyncCommitteeMessage_Optimistic(t *testing.T) {
Topic: &topic,
},
}
res, err := r.validateCommitteeIndexBeaconAttestation(ctx, "foobar", msg)
res, err := r.validateSyncCommitteeMessage(ctx, "foobar", msg)
Copy link
Member Author

Choose a reason for hiding this comment

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

These tests are bad to begin with. TestValidateSyncCommitteeMessage_Optimistic is calling validateCommitteeIndexBeaconAttestation. I fixed it here but they will be removed in #11320

assert.NoError(t, err)
valid := res == pubsub.ValidationIgnore
assert.Equal(t, true, valid, "Should have ignore this message")
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/sync/validate_sync_contribution_proof_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,7 +1054,7 @@ func TestValidateSyncContributionAndProof_Optimistic(t *testing.T) {
Topic: &topic,
},
}
res, err := r.validateCommitteeIndexBeaconAttestation(ctx, "foobar", msg)
res, err := r.validateSyncContributionAndProof(ctx, "foobar", msg)
assert.NoError(t, err)
valid := res == pubsub.ValidationIgnore
assert.Equal(t, true, valid, "Should have ignore this message")
Expand Down