From d3d7f67bec493870b64b94bef96fe1b570753e26 Mon Sep 17 00:00:00 2001 From: Potuz Date: Wed, 5 Nov 2025 12:54:38 -0500 Subject: [PATCH] Use head for block validation when possible (#15972) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Use head for block validation when possible When validating blocks for pubsub, we always copy a state and advance when we simply need to get a read only beacon state without a copy in most cases since the head state normally works. * fix test * fix tests * fix more tests * fix more tests * Add nil check to be safe * fix more tests * add test case 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --------- Co-authored-by: Claude --- beacon-chain/core/blocks/payload.go | 2 +- .../sync/pending_blocks_queue_test.go | 71 ++++++++---- beacon-chain/sync/validate_beacon_blocks.go | 94 ++++++++++++---- .../sync/validate_beacon_blocks_test.go | 103 +++++++++++++++++- changelog/potuz_use_head_block_validation.md | 3 + 5 files changed, 222 insertions(+), 51 deletions(-) create mode 100644 changelog/potuz_use_head_block_validation.md diff --git a/beacon-chain/core/blocks/payload.go b/beacon-chain/core/blocks/payload.go index 02dfa0ffb7..0d647025f8 100644 --- a/beacon-chain/core/blocks/payload.go +++ b/beacon-chain/core/blocks/payload.go @@ -86,7 +86,7 @@ func IsExecutionBlock(body interfaces.ReadOnlyBeaconBlockBody) (bool, error) { // def is_execution_enabled(state: BeaconState, body: ReadOnlyBeaconBlockBody) -> bool: // // return is_merge_block(state, body) or is_merge_complete(state) -func IsExecutionEnabled(st state.BeaconState, body interfaces.ReadOnlyBeaconBlockBody) (bool, error) { +func IsExecutionEnabled(st state.ReadOnlyBeaconState, body interfaces.ReadOnlyBeaconBlockBody) (bool, error) { if st == nil || body == nil { return false, errors.New("nil state or block body") } diff --git a/beacon-chain/sync/pending_blocks_queue_test.go b/beacon-chain/sync/pending_blocks_queue_test.go index 92628d57c0..ab0dfe78a7 100644 --- a/beacon-chain/sync/pending_blocks_queue_test.go +++ b/beacon-chain/sync/pending_blocks_queue_test.go @@ -43,15 +43,16 @@ func TestRegularSyncBeaconBlockSubscriber_ProcessPendingBlocks1(t *testing.T) { db := dbtest.SetupDB(t) p1 := p2ptest.NewTestP2P(t) + mockChain := &mock.ChainService{ + FinalizedCheckPoint: ðpb.Checkpoint{ + Epoch: 0, + }, + } r := &Service{ cfg: &config{ p2p: p1, beaconDB: db, - chain: &mock.ChainService{ - FinalizedCheckPoint: ðpb.Checkpoint{ - Epoch: 0, - }, - }, + chain: mockChain, clock: startup.NewClock(time.Unix(0, 0), [32]byte{}), stateGen: stategen.New(db, doublylinkedtree.New()), }, @@ -64,6 +65,12 @@ func TestRegularSyncBeaconBlockSubscriber_ProcessPendingBlocks1(t *testing.T) { util.SaveBlock(t, t.Context(), r.cfg.beaconDB, b0) b0Root, err := b0.Block.HashTreeRoot() require.NoError(t, err) + + // Setup head state for blockVerifyingState logic + st, err := util.NewBeaconState() + require.NoError(t, err) + mockChain.Root = b0Root[:] + mockChain.State = st b3 := util.NewBeaconBlock() b3.Block.Slot = 3 b3.Block.ParentRoot = b0Root[:] @@ -115,16 +122,17 @@ func TestRegularSyncBeaconBlockSubscriber_OptimisticStatus(t *testing.T) { db := dbtest.SetupDB(t) p1 := p2ptest.NewTestP2P(t) + mockChain := &mock.ChainService{ + Optimistic: true, + FinalizedCheckPoint: ðpb.Checkpoint{ + Epoch: 0, + }, + } r := &Service{ cfg: &config{ p2p: p1, beaconDB: db, - chain: &mock.ChainService{ - Optimistic: true, - FinalizedCheckPoint: ðpb.Checkpoint{ - Epoch: 0, - }, - }, + chain: mockChain, clock: startup.NewClock(time.Unix(0, 0), [32]byte{}), stateGen: stategen.New(db, doublylinkedtree.New()), }, @@ -137,6 +145,12 @@ func TestRegularSyncBeaconBlockSubscriber_OptimisticStatus(t *testing.T) { util.SaveBlock(t, t.Context(), r.cfg.beaconDB, b0) b0Root, err := b0.Block.HashTreeRoot() require.NoError(t, err) + + // Setup head state for blockVerifyingState logic + st, err := util.NewBeaconState() + require.NoError(t, err) + mockChain.Root = b0Root[:] + mockChain.State = st b3 := util.NewBeaconBlock() b3.Block.Slot = 3 b3.Block.ParentRoot = b0Root[:] @@ -189,16 +203,17 @@ func TestRegularSyncBeaconBlockSubscriber_ExecutionEngineTimesOut(t *testing.T) p1 := p2ptest.NewTestP2P(t) fcs := doublylinkedtree.New() + mockChain := &mock.ChainService{ + FinalizedCheckPoint: ðpb.Checkpoint{ + Epoch: 0, + }, + ReceiveBlockMockErr: execution.ErrHTTPTimeout, + } r := &Service{ cfg: &config{ p2p: p1, beaconDB: db, - chain: &mock.ChainService{ - FinalizedCheckPoint: ðpb.Checkpoint{ - Epoch: 0, - }, - ReceiveBlockMockErr: execution.ErrHTTPTimeout, - }, + chain: mockChain, clock: startup.NewClock(time.Unix(0, 0), [32]byte{}), stateGen: stategen.New(db, fcs), }, @@ -211,6 +226,12 @@ func TestRegularSyncBeaconBlockSubscriber_ExecutionEngineTimesOut(t *testing.T) util.SaveBlock(t, t.Context(), r.cfg.beaconDB, b0) b0Root, err := b0.Block.HashTreeRoot() require.NoError(t, err) + + // Setup head state for blockVerifyingState logic + st, err := util.NewBeaconState() + require.NoError(t, err) + mockChain.Root = b0Root[:] + mockChain.State = st b3 := util.NewBeaconBlock() b3.Block.Slot = 3 b3.Block.ParentRoot = b0Root[:] @@ -412,6 +433,14 @@ func TestRegularSyncBeaconBlockSubscriber_ProcessPendingBlocks_2Chains(t *testin util.SaveBlock(t, t.Context(), r.cfg.beaconDB, b0) b0Root, err := b0.Block.HashTreeRoot() require.NoError(t, err) + + // Setup head state for blockVerifyingState logic + st, err := util.NewBeaconState() + require.NoError(t, err) + mockChain := r.cfg.chain.(*mock.ChainService) + mockChain.Root = b0Root[:] + mockChain.State = st + b1 := util.NewBeaconBlock() b1.Block.Slot = 1 b1.Block.ParentRoot = b0Root[:] @@ -741,10 +770,8 @@ func TestService_ProcessPendingBlockOnCorrectSlot(t *testing.T) { proposerIdx, err := helpers.BeaconProposerIndex(ctx, copied) require.NoError(t, err) - st, err := util.NewBeaconState() - require.NoError(t, err) mockChain.Root = bRoot[:] - mockChain.State = st + mockChain.State = beaconState b1 := util.NewBeaconBlock() b1.Block.ParentRoot = bRoot[:] @@ -819,10 +846,8 @@ func TestService_ProcessBadPendingBlocks(t *testing.T) { proposerIdx, err := helpers.BeaconProposerIndex(ctx, copied) require.NoError(t, err) - st, err := util.NewBeaconState() - require.NoError(t, err) mockChain.Root = bRoot[:] - mockChain.State = st + mockChain.State = beaconState b1 := util.NewBeaconBlock() b1.Block.ParentRoot = bRoot[:] diff --git a/beacon-chain/sync/validate_beacon_blocks.go b/beacon-chain/sync/validate_beacon_blocks.go index 8418ff9819..07bba00446 100644 --- a/beacon-chain/sync/validate_beacon_blocks.go +++ b/beacon-chain/sync/validate_beacon_blocks.go @@ -1,6 +1,7 @@ package sync import ( + "bytes" "context" "fmt" "time" @@ -105,10 +106,6 @@ func (s *Service) validateBeaconBlockPubSub(ctx context.Context, pid peer.ID, ms }() } - if err := validateDenebBeaconBlock(blk.Block()); err != nil { - return pubsub.ValidationReject, err - } - // Verify the block is the first block received for the proposer for the slot. if s.hasSeenBlockIndexSlot(blk.Block().Slot(), blk.Block().ProposerIndex()) { // Attempt to detect and broadcast equivocation before ignoring @@ -262,12 +259,15 @@ func (s *Service) validateBeaconBlock(ctx context.Context, blk interfaces.ReadOn return err } - parentState, err := s.validatePhase0Block(ctx, blk, blockRoot) + verifyingState, err := s.validatePhase0Block(ctx, blk, blockRoot) if err != nil { return err } + if verifyingState == nil { + return errors.New("could not get verifying state") + } - if err = s.validateBellatrixBeaconBlock(ctx, parentState, blk.Block()); err != nil { + if err = s.validateBellatrixBeaconBlock(ctx, verifyingState, blk.Block()); err != nil { if errors.Is(err, ErrOptimisticParent) { return err } @@ -282,31 +282,25 @@ func (s *Service) validateBeaconBlock(ctx context.Context, blk interfaces.ReadOn // - Checks that the parent is in our forkchoice tree. // - Validates that the proposer signature is valid. // - Validates that the proposer index is valid. -func (s *Service) validatePhase0Block(ctx context.Context, blk interfaces.ReadOnlySignedBeaconBlock, blockRoot [32]byte) (state.BeaconState, error) { +// Returns a state that has compatible Randao Mix and active validator indices as the block's parent state advanced to the block's slot. +// This state can be used for further block validations. +func (s *Service) validatePhase0Block(ctx context.Context, blk interfaces.ReadOnlySignedBeaconBlock, blockRoot [32]byte) (state.ReadOnlyBeaconState, error) { if !s.cfg.chain.InForkchoice(blk.Block().ParentRoot()) { s.setBadBlock(ctx, blockRoot) return nil, blockchain.ErrNotDescendantOfFinalized } - parentState, err := s.cfg.stateGen.StateByRoot(ctx, blk.Block().ParentRoot()) + verifyingState, err := s.blockVerifyingState(ctx, blk) if err != nil { return nil, err } - - if err := blocks.VerifyBlockSignatureUsingCurrentFork(parentState, blk, blockRoot); err != nil { + if err := blocks.VerifyBlockSignatureUsingCurrentFork(verifyingState, blk, blockRoot); err != nil { if errors.Is(err, blocks.ErrInvalidSignature) { s.setBadBlock(ctx, blockRoot) } return nil, err } - // In the event the block is more than an epoch ahead from its - // parent state, we have to advance the state forward. - parentRoot := blk.Block().ParentRoot() - parentState, err = transition.ProcessSlotsUsingNextSlotCache(ctx, parentState, parentRoot[:], blk.Block().Slot()) - if err != nil { - return nil, err - } - idx, err := helpers.BeaconProposerIndex(ctx, parentState) + idx, err := helpers.BeaconProposerIndexAtSlot(ctx, verifyingState, blk.Block().Slot()) if err != nil { return nil, err } @@ -314,7 +308,59 @@ func (s *Service) validatePhase0Block(ctx context.Context, blk interfaces.ReadOn s.setBadBlock(ctx, blockRoot) return nil, errors.New("incorrect proposer index") } - return parentState, nil + return verifyingState, nil +} + +// blockVerifyingState returns the appropriate state to verify the signature and proposer index of the given block. +// The returned state is guaranteed to be at the same epoch as the block's epoch, and have the same randao mix and active validator indices as the +// block's parent state advanced to the block's slot. +func (s *Service) blockVerifyingState(ctx context.Context, blk interfaces.ReadOnlySignedBeaconBlock) (state.ReadOnlyBeaconState, error) { + headRoot, err := s.cfg.chain.HeadRoot(ctx) + if err != nil { + return nil, err + } + parentRoot := blk.Block().ParentRoot() + blockSlot := blk.Block().Slot() + blockEpoch := slots.ToEpoch(blockSlot) + headSlot := s.cfg.chain.HeadSlot() + headEpoch := slots.ToEpoch(headSlot) + // Use head if it's the parent + if bytes.Equal(parentRoot[:], headRoot) { + // If they are in the same epoch, then we can return the head state directly + if blockEpoch == headEpoch { + return s.cfg.chain.HeadStateReadOnly(ctx) + } + // Otherwise, we need to process the head state to the block's slot + headState, err := s.cfg.chain.HeadState(ctx) + if err != nil { + return nil, err + } + return transition.ProcessSlotsUsingNextSlotCache(ctx, headState, headRoot, blockSlot) + } + // If head and block are in the same epoch and head is compatible with the parent's target, then use head + if blockEpoch == headEpoch { + headTarget, err := s.cfg.chain.TargetRootForEpoch([32]byte(headRoot), blockEpoch) + if err != nil { + return nil, err + } + parentTarget, err := s.cfg.chain.TargetRootForEpoch([32]byte(parentRoot), blockEpoch) + if err != nil { + return nil, err + } + if bytes.Equal(headTarget[:], parentTarget[:]) { + return s.cfg.chain.HeadStateReadOnly(ctx) + } + } + // Otherwise retrieve the the parent state and advance it to the block's slot + parentState, err := s.cfg.stateGen.StateByRoot(ctx, parentRoot) + if err != nil { + return nil, err + } + parentEpoch := slots.ToEpoch(parentState.Slot()) + if blockEpoch == parentEpoch { + return parentState, nil + } + return transition.ProcessSlotsUsingNextSlotCache(ctx, parentState, parentRoot[:], blockSlot) } func validateDenebBeaconBlock(blk interfaces.ReadOnlyBeaconBlock) error { @@ -336,6 +382,8 @@ func validateDenebBeaconBlock(blk interfaces.ReadOnlyBeaconBlock) error { } // validateBellatrixBeaconBlock validates the block for the Bellatrix fork. +// The verifying state is used only to check if the chain is execution enabled. +// // spec code: // // If the execution is enabled for the block -- i.e. is_execution_enabled(state, block.body) then validate the following: @@ -348,14 +396,14 @@ func validateDenebBeaconBlock(blk interfaces.ReadOnlyBeaconBlock) error { // otherwise: // [IGNORE] The block's parent (defined by block.parent_root) passes all validation (including execution // node verification of the block.body.execution_payload). -func (s *Service) validateBellatrixBeaconBlock(ctx context.Context, parentState state.BeaconState, blk interfaces.ReadOnlyBeaconBlock) error { +func (s *Service) validateBellatrixBeaconBlock(ctx context.Context, verifyingState state.ReadOnlyBeaconState, blk interfaces.ReadOnlyBeaconBlock) error { // Error if block and state are not the same version - if parentState.Version() != blk.Version() { + if verifyingState.Version() != blk.Version() { return errors.New("block and state are not the same version") } body := blk.Body() - executionEnabled, err := blocks.IsExecutionEnabled(parentState, body) + executionEnabled, err := blocks.IsExecutionEnabled(verifyingState, body) if err != nil { return err } @@ -363,7 +411,7 @@ func (s *Service) validateBellatrixBeaconBlock(ctx context.Context, parentState return nil } - t, err := slots.StartTime(parentState.GenesisTime(), blk.Slot()) + t, err := slots.StartTime(verifyingState.GenesisTime(), blk.Slot()) if err != nil { return err } diff --git a/beacon-chain/sync/validate_beacon_blocks_test.go b/beacon-chain/sync/validate_beacon_blocks_test.go index a64aea417e..c11b13ffc6 100644 --- a/beacon-chain/sync/validate_beacon_blocks_test.go +++ b/beacon-chain/sync/validate_beacon_blocks_test.go @@ -73,7 +73,9 @@ func TestValidateBeaconBlockPubSub_InvalidSignature(t *testing.T) { Epoch: 0, Root: make([]byte, 32), }, - DB: db, + DB: db, + State: beaconState, + Root: bRoot[:], } r := &Service{ cfg: &config{ @@ -137,7 +139,9 @@ func TestValidateBeaconBlockPubSub_InvalidSignature_MarksBlockAsBad(t *testing.T Epoch: 0, Root: make([]byte, 32), }, - DB: db, + DB: db, + State: beaconState, + Root: bRoot[:], } r := &Service{ cfg: &config{ @@ -1301,7 +1305,10 @@ func TestValidateBeaconBlockPubSub_ValidExecutionPayload(t *testing.T) { FinalizedCheckPoint: ðpb.Checkpoint{ Epoch: 0, Root: make([]byte, 32), - }} + }, + State: beaconState, + Root: bRoot[:], + } r := &Service{ cfg: &config{ beaconDB: db, @@ -1536,7 +1543,10 @@ func Test_validateBeaconBlockProcessingWhenParentIsOptimistic(t *testing.T) { FinalizedCheckPoint: ðpb.Checkpoint{ Epoch: 0, Root: make([]byte, 32), - }} + }, + State: beaconState, + Root: bRoot[:], + } r := &Service{ cfg: &config{ beaconDB: db, @@ -1814,3 +1824,88 @@ func TestDetectAndBroadcastEquivocation(t *testing.T) { require.ErrorIs(t, err, ErrSlashingSignatureFailure) }) } + +func TestBlockVerifyingState_SameEpochAsParent(t *testing.T) { + ctx := t.Context() + db := dbtest.SetupDB(t) + + // Create a genesis state + beaconState, _ := util.DeterministicGenesisState(t, 100) + + // Create parent block at slot 1 + parentBlock := util.NewBeaconBlock() + parentBlock.Block.Slot = 1 + util.SaveBlock(t, ctx, db, parentBlock) + parentRoot, err := parentBlock.Block.HashTreeRoot() + require.NoError(t, err) + + // Save parent state at slot 1 (epoch 0) + parentState := beaconState.Copy() + require.NoError(t, parentState.SetSlot(1)) + require.NoError(t, db.SaveState(ctx, parentState, parentRoot)) + require.NoError(t, db.SaveStateSummary(ctx, ðpb.StateSummary{Root: parentRoot[:]})) + + // Create a different head block at a later epoch + headBlock := util.NewBeaconBlock() + headBlock.Block.Slot = 40 // Different epoch (epoch 1) + headBlock.Block.ParentRoot = parentRoot[:] // Head descends from parent + util.SaveBlock(t, ctx, db, headBlock) + headRoot, err := headBlock.Block.HashTreeRoot() + require.NoError(t, err) + + headState := beaconState.Copy() + require.NoError(t, headState.SetSlot(40)) + require.NoError(t, db.SaveState(ctx, headState, headRoot)) + + // Create a block at slot 2 (same epoch 0 as parent) + block := util.NewBeaconBlock() + block.Block.Slot = 2 + block.Block.ParentRoot = parentRoot[:] + signedBlock, err := blocks.NewSignedBeaconBlock(block) + require.NoError(t, err) + + forkchoiceStore := doublylinkedtree.New() + stateGen := stategen.New(db, forkchoiceStore) + + // Insert parent block into forkchoice + signedParentBlock, err := blocks.NewSignedBeaconBlock(parentBlock) + require.NoError(t, err) + roParentBlock, err := blocks.NewROBlockWithRoot(signedParentBlock, parentRoot) + require.NoError(t, err) + require.NoError(t, forkchoiceStore.InsertNode(ctx, parentState, roParentBlock)) + + // Insert head block into forkchoice + signedHeadBlock, err := blocks.NewSignedBeaconBlock(headBlock) + require.NoError(t, err) + roHeadBlock, err := blocks.NewROBlockWithRoot(signedHeadBlock, headRoot) + require.NoError(t, err) + require.NoError(t, forkchoiceStore.InsertNode(ctx, headState, roHeadBlock)) + + chainService := &mock.ChainService{ + DB: db, + Root: headRoot[:], // Head is different from parent + State: headState, // Set head state so HeadSlot() returns correct value + FinalizedCheckPoint: ðpb.Checkpoint{ + Epoch: 0, + Root: parentRoot[:], + }, + ForkChoiceStore: forkchoiceStore, + } + + r := &Service{ + cfg: &config{ + beaconDB: db, + chain: chainService, + stateGen: stateGen, + }, + } + + // Call blockVerifyingState - should return parent state without processing + result, err := r.blockVerifyingState(ctx, signedBlock) + require.NoError(t, err) + require.NotNil(t, result) + + // Verify that the returned state is at slot 1 (parent state slot) + // This confirms that the branch at line 361 was taken (returning parentState directly) + assert.Equal(t, primitives.Slot(1), result.Slot()) +} diff --git a/changelog/potuz_use_head_block_validation.md b/changelog/potuz_use_head_block_validation.md new file mode 100644 index 0000000000..29988d3891 --- /dev/null +++ b/changelog/potuz_use_head_block_validation.md @@ -0,0 +1,3 @@ +### Changed + +- Use head state for block pubsub validation when possible.