diff --git a/beacon-chain/blockchain/error.go b/beacon-chain/blockchain/error.go index 3be6076534..ae689fb8bb 100644 --- a/beacon-chain/blockchain/error.go +++ b/beacon-chain/blockchain/error.go @@ -41,13 +41,15 @@ var ( type invalidBlock struct { invalidAncestorRoots [][32]byte error - root [32]byte + root [32]byte + lastValidHash [32]byte } type invalidBlockError interface { Error() string InvalidAncestorRoots() [][32]byte BlockRoot() [32]byte + LastValidHash() [32]byte } // BlockRoot returns the invalid block root. @@ -55,6 +57,11 @@ func (e invalidBlock) BlockRoot() [32]byte { return e.root } +// LastValidHash returns the last valid hash root. +func (e invalidBlock) LastValidHash() [32]byte { + return e.lastValidHash +} + // InvalidAncestorRoots returns an optional list of invalid roots of the invalid block which leads up last valid root. func (e invalidBlock) InvalidAncestorRoots() [][32]byte { return e.invalidAncestorRoots @@ -72,6 +79,19 @@ func IsInvalidBlock(e error) bool { return true } +// InvalidBlockLVH returns the invalid block last valid hash root. If the error +// doesn't have a last valid hash, [32]byte{} is returned. +func InvalidBlockLVH(e error) [32]byte { + if e == nil { + return [32]byte{} + } + d, ok := e.(invalidBlockError) + if !ok { + return [32]byte{} + } + return d.LastValidHash() +} + // InvalidBlockRoot returns the invalid block root. If the error // doesn't have an invalid blockroot. [32]byte{} is returned. func InvalidBlockRoot(e error) [32]byte { diff --git a/beacon-chain/blockchain/execution_engine.go b/beacon-chain/blockchain/execution_engine.go index 49ebe6cefc..45566226b4 100644 --- a/beacon-chain/blockchain/execution_engine.go +++ b/beacon-chain/blockchain/execution_engine.go @@ -220,35 +220,37 @@ func (s *Service) notifyNewPayload(ctx context.Context, postStateVersion int, }).Info("Called new payload with optimistic block") return false, nil case execution.ErrInvalidPayloadStatus: - newPayloadInvalidNodeCount.Inc() - root, err := blk.Block().HashTreeRoot() - if err != nil { - return false, err - } - invalidRoots, err := s.cfg.ForkChoiceStore.SetOptimisticToInvalid(ctx, root, blk.Block().ParentRoot(), bytesutil.ToBytes32(lastValidHash)) - if err != nil { - return false, err - } - if err := s.removeInvalidBlockAndState(ctx, invalidRoots); err != nil { - return false, err - } - log.WithFields(logrus.Fields{ - "slot": blk.Block().Slot(), - "blockRoot": fmt.Sprintf("%#x", root), - "invalidChildrenCount": len(invalidRoots), - }).Warn("Pruned invalid blocks") + lvh := bytesutil.ToBytes32(lastValidHash) return false, invalidBlock{ - invalidAncestorRoots: invalidRoots, - error: ErrInvalidPayload, + error: ErrInvalidPayload, + lastValidHash: lvh, } - case execution.ErrInvalidBlockHashPayloadStatus: - newPayloadInvalidNodeCount.Inc() - return false, ErrInvalidBlockHashPayloadStatus default: return false, errors.WithMessage(ErrUndefinedExecutionEngineError, err.Error()) } } +// reportInvalidBlock deals with the event that an invalid block was detected by the execution layer +func (s *Service) reportInvalidBlock(ctx context.Context, root, parentRoot, lvh [32]byte) error { + newPayloadInvalidNodeCount.Inc() + invalidRoots, err := s.cfg.ForkChoiceStore.SetOptimisticToInvalid(ctx, root, parentRoot, lvh) + if err != nil { + return err + } + if err := s.removeInvalidBlockAndState(ctx, invalidRoots); err != nil { + return err + } + log.WithFields(logrus.Fields{ + "blockRoot": fmt.Sprintf("%#x", root), + "invalidChildrenCount": len(invalidRoots), + }).Warn("Pruned invalid blocks") + return invalidBlock{ + invalidAncestorRoots: invalidRoots, + error: ErrInvalidPayload, + lastValidHash: lvh, + } +} + // getPayloadAttributes returns the payload attributes for the given state and slot. // The attribute is required to initiate a payload build process in the context of an `engine_forkchoiceUpdated` call. func (s *Service) getPayloadAttribute(ctx context.Context, st state.BeaconState, slot primitives.Slot, headRoot []byte) (bool, payloadattribute.Attributer, primitives.ValidatorIndex) { diff --git a/beacon-chain/blockchain/execution_engine_test.go b/beacon-chain/blockchain/execution_engine_test.go index 11b8842e0e..6fa81abc46 100644 --- a/beacon-chain/blockchain/execution_engine_test.go +++ b/beacon-chain/blockchain/execution_engine_test.go @@ -743,6 +743,37 @@ func Test_NotifyNewPayload_SetOptimisticToValid(t *testing.T) { require.Equal(t, true, validated) } +func Test_reportInvalidBlock(t *testing.T) { + params.SetupTestConfigCleanup(t) + params.OverrideBeaconConfig(params.MainnetConfig()) + service, tr := minimalTestService(t) + ctx, _, fcs := tr.ctx, tr.db, tr.fcs + jcp := ðpb.Checkpoint{} + st, root, err := prepareForkchoiceState(ctx, 0, [32]byte{'A'}, [32]byte{}, [32]byte{'a'}, jcp, jcp) + require.NoError(t, err) + require.NoError(t, fcs.InsertNode(ctx, st, root)) + st, root, err = prepareForkchoiceState(ctx, 1, [32]byte{'B'}, [32]byte{'A'}, [32]byte{'b'}, jcp, jcp) + require.NoError(t, err) + require.NoError(t, fcs.InsertNode(ctx, st, root)) + st, root, err = prepareForkchoiceState(ctx, 2, [32]byte{'C'}, [32]byte{'B'}, [32]byte{'c'}, jcp, jcp) + require.NoError(t, err) + require.NoError(t, fcs.InsertNode(ctx, st, root)) + + st, root, err = prepareForkchoiceState(ctx, 3, [32]byte{'D'}, [32]byte{'C'}, [32]byte{'d'}, jcp, jcp) + require.NoError(t, err) + require.NoError(t, fcs.InsertNode(ctx, st, root)) + + require.NoError(t, fcs.SetOptimisticToValid(ctx, [32]byte{'A'})) + err = service.reportInvalidBlock(ctx, [32]byte{'D'}, [32]byte{'C'}, [32]byte{'a'}) + require.Equal(t, IsInvalidBlock(err), true) + require.Equal(t, InvalidBlockLVH(err), [32]byte{'a'}) + invalidRoots := InvalidAncestorRoots(err) + require.Equal(t, 3, len(invalidRoots)) + require.Equal(t, [32]byte{'D'}, invalidRoots[0]) + require.Equal(t, [32]byte{'C'}, invalidRoots[1]) + require.Equal(t, [32]byte{'B'}, invalidRoots[2]) +} + func Test_GetPayloadAttribute(t *testing.T) { service, tr := minimalTestService(t, WithProposerIdsCache(cache.NewProposerPayloadIDsCache())) ctx := tr.ctx diff --git a/beacon-chain/blockchain/process_block.go b/beacon-chain/blockchain/process_block.go index 5dc9741bd6..d89f127b1f 100644 --- a/beacon-chain/blockchain/process_block.go +++ b/beacon-chain/blockchain/process_block.go @@ -107,7 +107,8 @@ func (s *Service) onBlock(ctx context.Context, signed interfaces.ReadOnlySignedB } // Verify that the parent block is in forkchoice - if !s.cfg.ForkChoiceStore.HasNode(b.ParentRoot()) { + parentRoot := b.ParentRoot() + if !s.cfg.ForkChoiceStore.HasNode(parentRoot) { return ErrNotDescendantOfFinalized } @@ -134,6 +135,9 @@ func (s *Service) onBlock(ctx context.Context, signed interfaces.ReadOnlySignedB } isValidPayload, err := s.notifyNewPayload(ctx, postStateVersion, postStateHeader, signed) if err != nil { + if IsInvalidBlock(err) && InvalidBlockLVH(err) != [32]byte{} { + return s.reportInvalidBlock(ctx, blockRoot, parentRoot, InvalidBlockLVH(err)) + } return errors.Wrap(err, "could not validate new payload") } if signed.Version() < version.Capella && isValidPayload {