Skip to content
Open
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
9 changes: 7 additions & 2 deletions beacon-chain/blockchain/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package blockchain

import (
stderrors "errors"
"fmt"

"github.com/OffchainLabs/prysm/v6/beacon-chain/verification"
fieldparams "github.com/OffchainLabs/prysm/v6/config/fieldparams"
"github.com/pkg/errors"
)

Expand All @@ -29,8 +31,6 @@ var (
errWSBlockNotFound = errors.New("weak subjectivity root not found in db")
// errWSBlockNotFoundInEpoch is returned when a block is not found in the WS cache or DB within epoch.
errWSBlockNotFoundInEpoch = errors.New("weak subjectivity root not found in db within epoch")
// ErrNotDescendantOfFinalized is returned when a block is not a descendant of the finalized checkpoint
ErrNotDescendantOfFinalized = invalidBlock{error: errors.New("not descendant of finalized checkpoint")}
// ErrNotCheckpoint is returned when a given checkpoint is not a
// checkpoint in any chain known to forkchoice
ErrNotCheckpoint = errors.New("not a checkpoint in forkchoice")
Expand All @@ -48,6 +48,11 @@ var (
errBlockBeingSynced = errors.New("block is being synced")
)

// ErrRootNotInForkchoice is returned when a root cannot be found in forkchoice.
func ErrRootNotInForkchoice(root [fieldparams.RootLength]byte) invalidBlock {
return invalidBlock{error: fmt.Errorf("root %#x not in forkchoice", root), root: root}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this? this is a very dangerous change.

Copy link
Contributor

Choose a reason for hiding this comment

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

nah, my bad, read this incorrectly, this is fine, reviewing.

}

// An invalid block is the block that fails state transition based on the core protocol rules.
// The beacon node shall not be accepting nor building blocks that branch off from an invalid block.
// Some examples of invalid blocks are:
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/blockchain/process_block_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ func (s *Service) fillInForkChoiceMissingBlocks(ctx context.Context, signed inte
return nil
}
if root != s.ensureRootNotZeros(finalized.Root) && !s.cfg.ForkChoiceStore.HasNode(root) {
return ErrNotDescendantOfFinalized
return ErrRootNotInForkchoice(root)
}
slices.Reverse(pendingNodes)
return s.cfg.ForkChoiceStore.InsertChain(ctx, pendingNodes)
Expand Down
3 changes: 1 addition & 2 deletions beacon-chain/blockchain/process_block_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ func TestFillForkChoiceMissingBlocks_FinalizedSibling(t *testing.T) {

err = service.fillInForkChoiceMissingBlocks(
t.Context(), wsb, beaconState.FinalizedCheckpoint(), beaconState.CurrentJustifiedCheckpoint())
require.Equal(t, ErrNotDescendantOfFinalized.Error(), err.Error())
require.Equal(t, ErrRootNotInForkchoice(bytesutil.ToBytes32(roots[8])), err.Error())
}

func TestFillForkChoiceMissingBlocks_ErrorCases(t *testing.T) {
Expand Down Expand Up @@ -3302,7 +3302,6 @@ func Test_postBlockProcess_EventSending(t *testing.T) {
}
}


func setupLightClientTestRequirements(ctx context.Context, t *testing.T, s *Service, v int, options ...util.LightClientOption) (*util.TestLightClient, *postBlockProcessConfig) {
var l *util.TestLightClient
switch v {
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/blockchain/receive_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ func (s *Service) validateStateTransition(ctx context.Context, preState state.Be
// Verify that the parent block is in forkchoice
parentRoot := b.ParentRoot()
if !s.InForkchoice(parentRoot) {
return nil, ErrNotDescendantOfFinalized
return nil, ErrRootNotInForkchoice(parentRoot)
}
stateTransitionStartTime := time.Now()
postState, err := transition.ExecuteStateTransition(ctx, preState, signed)
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/sync/pending_attestations_queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func (s *Service) processAttestationBucket(ctx context.Context, bucket *attestat

// Shared validations for the entire bucket.
if !s.cfg.chain.InForkchoice(bytesutil.ToBytes32(data.BeaconBlockRoot)) {
log.WithError(blockchain.ErrNotDescendantOfFinalized).WithField("root", fmt.Sprintf("%#x", data.BeaconBlockRoot)).Debug("Failed forkchoice check for bucket")
log.WithError(blockchain.ErrRootNotInForkchoice(bytesutil.ToBytes32(data.BeaconBlockRoot))).Debug("Failed forkchoice check for bucket")
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure why we need to return an invalid block and a root here.

return
}

Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/sync/validate_aggregate_proof.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,8 @@ func (s *Service) validateAggregatedAtt(ctx context.Context, signed ethpb.Signed

// Verify current finalized checkpoint is an ancestor of the block defined by the attestation's beacon block root.
if !s.cfg.chain.InForkchoice(bytesutil.ToBytes32(data.BeaconBlockRoot)) {
tracing.AnnotateError(span, blockchain.ErrNotDescendantOfFinalized)
return pubsub.ValidationIgnore, blockchain.ErrNotDescendantOfFinalized
tracing.AnnotateError(span, blockchain.ErrRootNotInForkchoice(bytesutil.ToBytes32(data.BeaconBlockRoot)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

return pubsub.ValidationIgnore, blockchain.ErrRootNotInForkchoice(bytesutil.ToBytes32(data.BeaconBlockRoot))
}

bs, err := s.cfg.chain.AttestationTargetState(ctx, data.Target)
Expand Down
4 changes: 2 additions & 2 deletions beacon-chain/sync/validate_beacon_attestation.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ func (s *Service) validateCommitteeIndexBeaconAttestation(
s.savePendingAtt(att)
}
if !s.cfg.chain.InForkchoice(blockRoot) {
tracing.AnnotateError(span, blockchain.ErrNotDescendantOfFinalized)
return pubsub.ValidationIgnore, blockchain.ErrNotDescendantOfFinalized
tracing.AnnotateError(span, blockchain.ErrRootNotInForkchoice(blockRoot))
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

return pubsub.ValidationIgnore, blockchain.ErrRootNotInForkchoice(blockRoot)
}
if err = s.cfg.chain.VerifyLmdFfgConsistency(ctx, att); err != nil {
tracing.AnnotateError(span, err)
Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/sync/validate_beacon_blocks.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ func (s *Service) validateBeaconBlock(ctx context.Context, blk interfaces.ReadOn
func (s *Service) validatePhase0Block(ctx context.Context, blk interfaces.ReadOnlySignedBeaconBlock, blockRoot [32]byte) (state.BeaconState, error) {
if !s.cfg.chain.InForkchoice(blk.Block().ParentRoot()) {
s.setBadBlock(ctx, blockRoot)
return nil, blockchain.ErrNotDescendantOfFinalized
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

return nil, blockchain.ErrRootNotInForkchoice(blk.Block().ParentRoot())
}

parentState, err := s.cfg.stateGen.StateByRoot(ctx, blk.Block().ParentRoot())
Expand Down
3 changes: 3 additions & 0 deletions changelog/james-prysm_change-p2p-forkchoice-error.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### Changed

- changed ErrNotDescendantOfFinalized to ErrRootNotInForkChoice.