From 419dbd57f7300d6edc8b3e3e16ce9de60262e609 Mon Sep 17 00:00:00 2001 From: Delweng Date: Mon, 9 Oct 2023 00:51:52 -0500 Subject: [PATCH] beacon-chain/execution: no need to reread and unmarshal the eth1Data twice (#12826) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * beacon-chain/execution: no need to reread and unmarshal the eth1Data multitimes Signed-off-by: jsvisa * beacon-chain/execution: rename to validPowchainData Signed-off-by: jsvisa * beacon-chain/execution: no return eth1Data if error Signed-off-by: jsvisa * beacon-chain/execution: return eth1data even if genstate is nil Signed-off-by: jsvisa --------- Signed-off-by: jsvisa Co-authored-by: RadosÅ‚aw Kapka Co-authored-by: Nishant Das --- beacon-chain/execution/service.go | 35 ++++++++++++-------------- beacon-chain/execution/service_test.go | 9 ++++--- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/beacon-chain/execution/service.go b/beacon-chain/execution/service.go index af5a518318e3..a5fb58148ff1 100644 --- a/beacon-chain/execution/service.go +++ b/beacon-chain/execution/service.go @@ -208,13 +208,9 @@ func NewService(ctx context.Context, opts ...Option) (*Service, error) { } } - if err := s.ensureValidPowchainData(ctx); err != nil { - return nil, errors.Wrap(err, "unable to validate powchain data") - } - - eth1Data, err := s.cfg.beaconDB.ExecutionChainData(ctx) + eth1Data, err := s.validPowchainData(ctx) if err != nil { - return nil, errors.Wrap(err, "unable to retrieve eth1 data") + return nil, errors.Wrap(err, "unable to validate powchain data") } if err := s.initializeEth1Data(ctx, eth1Data); err != nil { return nil, err @@ -822,23 +818,22 @@ func validateDepositContainers(ctrs []*ethpb.DepositContainer) bool { // Validates the current powchain data is saved and makes sure that any // embedded genesis state is correctly accounted for. -func (s *Service) ensureValidPowchainData(ctx context.Context) error { +func (s *Service) validPowchainData(ctx context.Context) (*ethpb.ETH1ChainData, error) { genState, err := s.cfg.beaconDB.GenesisState(ctx) if err != nil { - return err - } - // Exit early if no genesis state is saved. - if genState == nil || genState.IsNil() { - return nil + return nil, err } eth1Data, err := s.cfg.beaconDB.ExecutionChainData(ctx) if err != nil { - return errors.Wrap(err, "unable to retrieve eth1 data") + return nil, errors.Wrap(err, "unable to retrieve eth1 data") + } + if genState == nil || genState.IsNil() { + return eth1Data, nil } if eth1Data == nil || !eth1Data.ChainstartData.Chainstarted || !validateDepositContainers(eth1Data.DepositContainers) { pbState, err := native.ProtobufBeaconStatePhase0(s.preGenesisState.ToProtoUnsafe()) if err != nil { - return err + return nil, err } s.chainStartData = ðpb.ChainStartData{ Chainstarted: true, @@ -856,22 +851,24 @@ func (s *Service) ensureValidPowchainData(ctx context.Context) error { if features.Get().EnableEIP4881 { trie, ok := s.depositTrie.(*depositsnapshot.DepositTree) if !ok { - return errors.New("deposit trie was not EIP4881 DepositTree") + return nil, errors.New("deposit trie was not EIP4881 DepositTree") } eth1Data.DepositSnapshot, err = trie.ToProto() if err != nil { - return err + return nil, err } } else { trie, ok := s.depositTrie.(*trie.SparseMerkleTrie) if !ok { - return errors.New("deposit trie was not SparseMerkleTrie") + return nil, errors.New("deposit trie was not SparseMerkleTrie") } eth1Data.Trie = trie.ToProto() } - return s.cfg.beaconDB.SaveExecutionChainData(ctx, eth1Data) + if err := s.cfg.beaconDB.SaveExecutionChainData(ctx, eth1Data); err != nil { + return nil, err + } } - return nil + return eth1Data, nil } func dedupEndpoints(endpoints []string) []string { diff --git a/beacon-chain/execution/service_test.go b/beacon-chain/execution/service_test.go index a8b8f48a7101..7f5c7cdea83c 100644 --- a/beacon-chain/execution/service_test.go +++ b/beacon-chain/execution/service_test.go @@ -571,7 +571,8 @@ func TestService_EnsureConsistentPowchainData(t *testing.T) { assert.NoError(t, genState.SetSlot(1000)) require.NoError(t, s1.cfg.beaconDB.SaveGenesisData(context.Background(), genState)) - require.NoError(t, s1.ensureValidPowchainData(context.Background())) + _, err = s1.validPowchainData(context.Background()) + require.NoError(t, err) eth1Data, err := s1.cfg.beaconDB.ExecutionChainData(context.Background()) assert.NoError(t, err) @@ -601,7 +602,8 @@ func TestService_InitializeCorrectly(t *testing.T) { assert.NoError(t, genState.SetSlot(1000)) require.NoError(t, s1.cfg.beaconDB.SaveGenesisData(context.Background(), genState)) - require.NoError(t, s1.ensureValidPowchainData(context.Background())) + _, err = s1.validPowchainData(context.Background()) + require.NoError(t, err) eth1Data, err := s1.cfg.beaconDB.ExecutionChainData(context.Background()) assert.NoError(t, err) @@ -636,7 +638,8 @@ func TestService_EnsureValidPowchainData(t *testing.T) { DepositContainers: []*ethpb.DepositContainer{{Index: 1}}, }) require.NoError(t, err) - require.NoError(t, s1.ensureValidPowchainData(context.Background())) + _, err = s1.validPowchainData(context.Background()) + require.NoError(t, err) eth1Data, err := s1.cfg.beaconDB.ExecutionChainData(context.Background()) assert.NoError(t, err)