From 8fc4243c5da2e6852f462dffefe229a76b5831ae Mon Sep 17 00:00:00 2001 From: Potuz Date: Sun, 3 Nov 2024 07:22:04 -0300 Subject: [PATCH] Use ROBlock earlier in the pipeline --- CHANGELOG.md | 1 + beacon-chain/blockchain/process_block_test.go | 12 ++++++--- beacon-chain/blockchain/receive_block.go | 27 ++++++++++--------- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4c4ca276cb..6d32badecc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve - Simplified `ExitedValidatorIndices`. - Simplified `EjectedValidatorIndices`. - `engine_newPayloadV4`,`engine_getPayloadV4` are changes due to new execution request serialization decisions, [PR](https://github.com/prysmaticlabs/prysm/pull/14580) +- Use ROBlock earlier in block syncing pipeline. ### Deprecated diff --git a/beacon-chain/blockchain/process_block_test.go b/beacon-chain/blockchain/process_block_test.go index 7ed483861f1..053eb5fc478 100644 --- a/beacon-chain/blockchain/process_block_test.go +++ b/beacon-chain/blockchain/process_block_test.go @@ -1520,7 +1520,9 @@ func TestStore_NoViableHead_NewPayload(t *testing.T) { require.NoError(t, err) preStateVersion, preStateHeader, err := getStateVersionAndPayload(preState) require.NoError(t, err) - _, err = service.validateExecutionOnBlock(ctx, preStateVersion, preStateHeader, wsb, root) + rowsb, err := consensusblocks.NewROBlockWithRoot(wsb, root) + require.NoError(t, err) + _, err = service.validateExecutionOnBlock(ctx, preStateVersion, preStateHeader, rowsb) require.ErrorContains(t, "received an INVALID payload from execution engine", err) // Check that forkchoice's head and store's headroot are the previous head (since the invalid block did // not finish importing and it was never imported to forkchoice). Check @@ -1714,7 +1716,9 @@ func TestStore_NoViableHead_Liveness(t *testing.T) { require.NoError(t, err) preStateVersion, preStateHeader, err := getStateVersionAndPayload(preState) require.NoError(t, err) - _, err = service.validateExecutionOnBlock(ctx, preStateVersion, preStateHeader, wsb, root) + rowsb, err := consensusblocks.NewROBlockWithRoot(wsb, root) + require.NoError(t, err) + _, err = service.validateExecutionOnBlock(ctx, preStateVersion, preStateHeader, rowsb) require.ErrorContains(t, "received an INVALID payload from execution engine", err) // Check that forkchoice's head and store's headroot are the previous head (since the invalid block did @@ -1964,7 +1968,9 @@ func TestNoViableHead_Reboot(t *testing.T) { require.NoError(t, err) preStateVersion, preStateHeader, err := getStateVersionAndPayload(preState) require.NoError(t, err) - _, err = service.validateExecutionOnBlock(ctx, preStateVersion, preStateHeader, wsb, root) + rowsb, err := consensusblocks.NewROBlockWithRoot(wsb, root) + require.NoError(t, err) + _, err = service.validateExecutionOnBlock(ctx, preStateVersion, preStateHeader, rowsb) require.ErrorContains(t, "received an INVALID payload from execution engine", err) // Check that the headroot/state are not in DB and restart the node diff --git a/beacon-chain/blockchain/receive_block.go b/beacon-chain/blockchain/receive_block.go index e6afed07685..ff8c4d9187e 100644 --- a/beacon-chain/blockchain/receive_block.go +++ b/beacon-chain/blockchain/receive_block.go @@ -18,6 +18,7 @@ import ( "github.com/prysmaticlabs/prysm/v5/config/features" "github.com/prysmaticlabs/prysm/v5/consensus-types/blocks" consensus_blocks "github.com/prysmaticlabs/prysm/v5/consensus-types/blocks" + consensusblocks "github.com/prysmaticlabs/prysm/v5/consensus-types/blocks" "github.com/prysmaticlabs/prysm/v5/consensus-types/interfaces" "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" "github.com/prysmaticlabs/prysm/v5/encoding/bytesutil" @@ -84,7 +85,12 @@ func (s *Service) ReceiveBlock(ctx context.Context, block interfaces.ReadOnlySig } currentCheckpoints := s.saveCurrentCheckpoints(preState) - postState, isValidPayload, err := s.validateExecutionAndConsensus(ctx, preState, blockCopy, blockRoot) + roblock, err := consensus_blocks.NewROBlockWithRoot(blockCopy, blockRoot) + if err != nil { + return err + } + + postState, isValidPayload, err := s.validateExecutionAndConsensus(ctx, preState, roblock) if err != nil { return err } @@ -101,10 +107,6 @@ func (s *Service) ReceiveBlock(ctx context.Context, block interfaces.ReadOnlySig if err := s.savePostStateInfo(ctx, blockRoot, blockCopy, postState); err != nil { return errors.Wrap(err, "could not save post state info") } - roblock, err := consensus_blocks.NewROBlockWithRoot(blockCopy, blockRoot) - if err != nil { - return err - } args := &postBlockProcessConfig{ ctx: ctx, roblock: roblock, @@ -188,8 +190,7 @@ func (s *Service) updateCheckpoints( func (s *Service) validateExecutionAndConsensus( ctx context.Context, preState state.BeaconState, - block interfaces.SignedBeaconBlock, - blockRoot [32]byte, + block consensusblocks.ROBlock, ) (state.BeaconState, bool, error) { preStateVersion, preStateHeader, err := getStateVersionAndPayload(preState) if err != nil { @@ -208,7 +209,7 @@ func (s *Service) validateExecutionAndConsensus( var isValidPayload bool eg.Go(func() error { var err error - isValidPayload, err = s.validateExecutionOnBlock(ctx, preStateVersion, preStateHeader, block, blockRoot) + isValidPayload, err = s.validateExecutionOnBlock(ctx, preStateVersion, preStateHeader, block) if err != nil { return errors.Wrap(err, "could not notify the engine of the new payload") } @@ -559,16 +560,16 @@ func (s *Service) sendBlockAttestationsToSlasher(signed interfaces.ReadOnlySigne } // validateExecutionOnBlock notifies the engine of the incoming block execution payload and returns true if the payload is valid -func (s *Service) validateExecutionOnBlock(ctx context.Context, ver int, header interfaces.ExecutionData, signed interfaces.ReadOnlySignedBeaconBlock, blockRoot [32]byte) (bool, error) { - isValidPayload, err := s.notifyNewPayload(ctx, ver, header, signed) +func (s *Service) validateExecutionOnBlock(ctx context.Context, ver int, header interfaces.ExecutionData, block consensusblocks.ROBlock) (bool, error) { + isValidPayload, err := s.notifyNewPayload(ctx, ver, header, block) if err != nil { s.cfg.ForkChoiceStore.Lock() - err = s.handleInvalidExecutionError(ctx, err, blockRoot, signed.Block().ParentRoot()) + err = s.handleInvalidExecutionError(ctx, err, block.Root(), block.Block().ParentRoot()) s.cfg.ForkChoiceStore.Unlock() return false, err } - if signed.Version() < version.Capella && isValidPayload { - if err := s.validateMergeTransitionBlock(ctx, ver, header, signed); err != nil { + if block.Block().Version() < version.Capella && isValidPayload { + if err := s.validateMergeTransitionBlock(ctx, ver, header, block); err != nil { return isValidPayload, err } }