Don't ban blocks for context deadlines (#7040)

* Don't ban blocks for context deadlines
* Don't ban blocks for context deadlines
* Add test
* Merge branch 'master' into dont-ban-ctx-deadline2
* fmt
This commit is contained in:
Preston Van Loon 2020-08-17 18:21:10 -07:00 committed by GitHub
parent 9caa92cae4
commit 4d463c4a85
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 26 additions and 10 deletions

View File

@ -90,7 +90,7 @@ func (s *Service) processPendingBlocks(ctx context.Context) error {
if parentIsBad || blockIsBad {
// Set block as bad if its parent block is bad too.
if parentIsBad {
s.setBadBlock(blkRoot)
s.setBadBlock(ctx, blkRoot)
}
// Remove block from queue.
s.pendingQueueLock.Lock()
@ -142,7 +142,7 @@ func (s *Service) processPendingBlocks(ctx context.Context) error {
if err := s.chain.ReceiveBlock(ctx, b, blkRoot); err != nil {
log.Debugf("Could not process block from slot %d: %v", b.Block.Slot, err)
s.setBadBlock(blkRoot)
s.setBadBlock(ctx, blkRoot)
traceutil.AnnotateError(span, err)
}

View File

@ -32,7 +32,7 @@ func (s *Service) beaconBlockSubscriber(ctx context.Context, msg proto.Message)
if err := s.chain.ReceiveBlock(ctx, signed, root); err != nil {
interop.WriteBlockToDisk(signed, true /*failed*/)
s.setBadBlock(root)
s.setBadBlock(ctx, root)
return err
}

View File

@ -586,7 +586,7 @@ func TestValidateAggregateAndProof_BadBlock(t *testing.T) {
err = r.initCaches()
require.NoError(t, err)
// Set beacon block as bad.
r.setBadBlock(root)
r.setBadBlock(context.Background(), root)
buf := new(bytes.Buffer)
_, err = p.Encoding().EncodeGossip(buf, signedAggregateAndProof)
require.NoError(t, err)

View File

@ -52,7 +52,7 @@ func TestService_validateCommitteeIndexBeaconAttestation(t *testing.T) {
require.NoError(t, err)
invalidRoot := [32]byte{'A', 'B', 'C', 'D'}
s.setBadBlock(invalidRoot)
s.setBadBlock(ctx, invalidRoot)
digest, err := s.forkDigest()
require.NoError(t, err)

View File

@ -81,7 +81,7 @@ func (s *Service) validateBeaconBlockPubSub(ctx context.Context, pid peer.ID, ms
// Check if parent is a bad block and then reject the block.
if s.hasBadBlock(bytesutil.ToBytes32(blk.Block.ParentRoot)) {
log.Debugf("Received block with root %#x that has an invalid parent %#x", blockRoot, blk.Block.ParentRoot)
s.setBadBlock(blockRoot)
s.setBadBlock(ctx, blockRoot)
return pubsub.ValidationReject
}
@ -118,7 +118,7 @@ func (s *Service) validateBeaconBlockPubSub(ctx context.Context, pid peer.ID, ms
if err := s.chain.VerifyBlkDescendant(ctx, bytesutil.ToBytes32(blk.Block.ParentRoot)); err != nil {
log.WithError(err).Warn("Rejecting block")
s.setBadBlock(blockRoot)
s.setBadBlock(ctx, blockRoot)
return pubsub.ValidationReject
}
@ -136,7 +136,7 @@ func (s *Service) validateBeaconBlockPubSub(ctx context.Context, pid peer.ID, ms
if err := blocks.VerifyBlockSignature(parentState, blk); err != nil {
log.WithError(err).WithField("blockSlot", blk.Block.Slot).Warn("Could not verify block signature")
s.setBadBlock(blockRoot)
s.setBadBlock(ctx, blockRoot)
return pubsub.ValidationReject
}
@ -152,7 +152,7 @@ func (s *Service) validateBeaconBlockPubSub(ctx context.Context, pid peer.ID, ms
}
if blk.Block.ProposerIndex != idx {
log.WithError(err).WithField("blockSlot", blk.Block.Slot).Warn("Incorrect proposer index")
s.setBadBlock(blockRoot)
s.setBadBlock(ctx, blockRoot)
return pubsub.ValidationReject
}
@ -186,9 +186,12 @@ func (s *Service) hasBadBlock(root [32]byte) bool {
}
// Set bad block in the cache.
func (s *Service) setBadBlock(root [32]byte) {
func (s *Service) setBadBlock(ctx context.Context, root [32]byte) {
s.badBlockLock.Lock()
defer s.badBlockLock.Unlock()
if ctx.Err() != nil { // Do not mark block as bad if it was due to context error.
return
}
s.badBlockCache.Add(string(root[:]), true)
}

View File

@ -712,3 +712,16 @@ func TestValidateBeaconBlockPubSub_InvalidParentBlock(t *testing.T) {
result = r.validateBeaconBlockPubSub(ctx, "", m) == pubsub.ValidationAccept
assert.Equal(t, false, result)
}
func TestService_setBadBlock_DoesntSetWithContextErr(t *testing.T) {
s := Service{}
require.NoError(t, s.initCaches())
root := [32]byte{'b', 'a', 'd'}
ctx, cancel := context.WithCancel(context.Background())
cancel()
s.setBadBlock(ctx, root)
if s.hasBadBlock(root) {
t.Error("Set bad root with cancelled context")
}
}