Update execution_engine.go

* adding checksum check at validator client and beacon node

* adding validation and logs on validator client startup

* moving the log and validation

* fixing unit tests

* adding test for back checksum on validator client

* fixing bazel

* addressing comments

* fixing log display

* Update beacon-chain/node/config.go

* Apply suggestions from code review

* breaking up lines

* fixing unit test

* ugh another fix to unit test

Co-authored-by: Raul Jordan <raul@prysmaticlabs.com>
Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>

beacon-chain/blockchain/execution_engine.go

@@ -60,10 +60,9 @@ func (s *Service) notifyForkchoiceUpdate(ctx context.Context, arg *notifyForkcho
 		return nil, errors.Wrap(err, "could not get execution payload")
 	}
 	finalizedHash := s.store.FinalizedPayloadBlockHash()
-	justifiedHash := s.store.JustifiedPayloadBlockHash()
 	fcs := &enginev1.ForkchoiceState{
 		HeadBlockHash:      headPayload.BlockHash,
-		SafeBlockHash:      justifiedHash[:],
+		SafeBlockHash:      headPayload.BlockHash,
 		FinalizedBlockHash: finalizedHash[:],
 	}
 

beacon-chain/blockchain/process_block.go

@@ -123,7 +123,9 @@ func (s *Service) onBlock(ctx context.Context, signed interfaces.SignedBeaconBlo
 			return err
 		}
 	}
-
+	if err := s.savePostStateInfo(ctx, blockRoot, signed, postState); err != nil {
+		return err
+	}
 	if err := s.insertBlockAndAttestationsToForkChoiceStore(ctx, signed.Block(), blockRoot, postState); err != nil {
 		return errors.Wrapf(err, "could not insert block %d to fork choice store", signed.Block().Slot())
 	}
@@ -146,9 +148,6 @@ func (s *Service) onBlock(ctx context.Context, signed interfaces.SignedBeaconBlo
 		return err
 	}
 
-	if err := s.savePostStateInfo(ctx, blockRoot, signed, postState); err != nil {
-		return err
-	}
 	// If slasher is configured, forward the attestations in the block via
 	// an event feed for processing.
 	if features.Get().EnableSlasher {

beacon-chain/blockchain/process_block_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"math/big"
 	"strconv"
+	"sync"
 	"testing"
 	"time"
 
@@ -39,6 +40,7 @@ import (
 	"github.com/prysmaticlabs/prysm/testing/require"
 	"github.com/prysmaticlabs/prysm/testing/util"
 	prysmTime "github.com/prysmaticlabs/prysm/time"
+	logTest "github.com/sirupsen/logrus/hooks/test"
 )
 
 func TestStore_OnBlock_ProtoArray(t *testing.T) {
@@ -1891,3 +1893,82 @@ func TestService_insertSlashingsToForkChoiceStore(t *testing.T) {
 	require.NoError(t, err)
 	service.insertSlashingsToForkChoiceStore(ctx, wb.Block().Body().AttesterSlashings())
 }
+
+func TestOnBlock_ProcessBlocksParallel(t *testing.T) {
+	ctx := context.Background()
+	beaconDB := testDB.SetupDB(t)
+	fcs := protoarray.New(0, 0, [32]byte{'a'})
+	depositCache, err := depositcache.New()
+	require.NoError(t, err)
+	opts := []Option{
+		WithDatabase(beaconDB),
+		WithStateGen(stategen.New(beaconDB)),
+		WithForkChoiceStore(fcs),
+		WithDepositCache(depositCache),
+		WithStateNotifier(&mock.MockStateNotifier{}),
+	}
+	service, err := NewService(ctx, opts...)
+	require.NoError(t, err)
+
+	gs, keys := util.DeterministicGenesisState(t, 32)
+	require.NoError(t, service.saveGenesisData(ctx, gs))
+	gBlk, err := service.cfg.BeaconDB.GenesisBlock(ctx)
+	require.NoError(t, err)
+	gRoot, err := gBlk.Block().HashTreeRoot()
+	require.NoError(t, err)
+	service.store.SetFinalizedCheckptAndPayloadHash(&ethpb.Checkpoint{Root: gRoot[:]}, [32]byte{'a'})
+
+	blk1, err := util.GenerateFullBlock(gs, keys, util.DefaultBlockGenConfig(), 1)
+	require.NoError(t, err)
+	r1, err := blk1.Block.HashTreeRoot()
+	require.NoError(t, err)
+	wsb1, err := wrapper.WrappedSignedBeaconBlock(blk1)
+	require.NoError(t, err)
+	blk2, err := util.GenerateFullBlock(gs, keys, util.DefaultBlockGenConfig(), 2)
+	require.NoError(t, err)
+	r2, err := blk2.Block.HashTreeRoot()
+	require.NoError(t, err)
+	wsb2, err := wrapper.WrappedSignedBeaconBlock(blk2)
+	require.NoError(t, err)
+	blk3, err := util.GenerateFullBlock(gs, keys, util.DefaultBlockGenConfig(), 3)
+	require.NoError(t, err)
+	r3, err := blk3.Block.HashTreeRoot()
+	require.NoError(t, err)
+	wsb3, err := wrapper.WrappedSignedBeaconBlock(blk3)
+	require.NoError(t, err)
+	blk4, err := util.GenerateFullBlock(gs, keys, util.DefaultBlockGenConfig(), 4)
+	require.NoError(t, err)
+	r4, err := blk4.Block.HashTreeRoot()
+	require.NoError(t, err)
+	wsb4, err := wrapper.WrappedSignedBeaconBlock(blk4)
+	require.NoError(t, err)
+
+	logHook := logTest.NewGlobal()
+	for i := 0; i < 10; i++ {
+		var wg sync.WaitGroup
+		wg.Add(4)
+		go func() {
+			require.NoError(t, service.onBlock(ctx, wsb1, r1))
+			wg.Done()
+		}()
+		go func() {
+			require.NoError(t, service.onBlock(ctx, wsb2, r2))
+			wg.Done()
+		}()
+		go func() {
+			require.NoError(t, service.onBlock(ctx, wsb3, r3))
+			wg.Done()
+		}()
+		go func() {
+			require.NoError(t, service.onBlock(ctx, wsb4, r4))
+			wg.Done()
+		}()
+		wg.Wait()
+		require.LogsDoNotContain(t, logHook, "New head does not exist in DB. Do nothing")
+		require.NoError(t, service.cfg.BeaconDB.DeleteBlock(ctx, r1))
+		require.NoError(t, service.cfg.BeaconDB.DeleteBlock(ctx, r2))
+		require.NoError(t, service.cfg.BeaconDB.DeleteBlock(ctx, r3))
+		require.NoError(t, service.cfg.BeaconDB.DeleteBlock(ctx, r4))
+		service.cfg.ForkChoiceStore = protoarray.New(0, 0, [32]byte{'a'})
+	}
+}

beacon-chain/node/config.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/pkg/errors"
 	"github.com/prysmaticlabs/prysm/cmd"
 	"github.com/prysmaticlabs/prysm/cmd/beacon-chain/flags"
 	"github.com/prysmaticlabs/prysm/config/params"
@@ -117,7 +118,18 @@ func configureExecutionSetting(cliCtx *cli.Context) error {
 	if !common.IsHexAddress(ha) {
 		return fmt.Errorf("%s is not a valid fee recipient address", ha)
 	}
-	c.DefaultFeeRecipient = common.HexToAddress(ha)
+	mixedcaseAddress, err := common.NewMixedcaseAddressFromString(ha)
+	if err != nil {
+		return errors.Wrapf(err, "could not decode fee recipient %s", ha)
+	}
+	checksumAddress := common.HexToAddress(ha)
+	if !mixedcaseAddress.ValidChecksum() {
+		log.Warnf("Fee recipient %s is not a checksum Ethereum address. "+
+			"The checksummed address is %s and will be used as the fee recipient. "+
+			"We recommend using a mixed-case address (checksum) "+
+			"to prevent spelling mistakes in your fee recipient Ethereum address", ha, checksumAddress.Hex())
+	}
+	c.DefaultFeeRecipient = checksumAddress
 	params.OverrideBeaconConfig(c)
 	return nil
 }

beacon-chain/node/config_test.go

@@ -87,6 +87,7 @@ func TestConfigureProofOfWork(t *testing.T) {
 
 func TestConfigureExecutionSetting(t *testing.T) {
 	params.SetupTestConfigCleanup(t)
+	hook := logTest.NewGlobal()
 
 	app := cli.App{}
 	set := flag.NewFlagSet("test", 0)
@@ -102,11 +103,15 @@ func TestConfigureExecutionSetting(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, common.HexToAddress("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"), params.BeaconConfig().DefaultFeeRecipient)
 
-	require.NoError(t, set.Set(flags.SuggestedFeeRecipient.Name, "0xAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"))
+	assert.LogsContain(t, hook,
+		"is not a checksum Ethereum address",
+	)
+	require.NoError(t, set.Set(flags.SuggestedFeeRecipient.Name, "0xaAaAaAaaAaAaAaaAaAAAAAAAAaaaAaAaAaaAaaAa"))
 	cliCtx = cli.NewContext(&app, set, nil)
 	err = configureExecutionSetting(cliCtx)
 	require.NoError(t, err)
-	assert.Equal(t, common.HexToAddress("0xAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"), params.BeaconConfig().DefaultFeeRecipient)
+	assert.Equal(t, common.HexToAddress("0xaAaAaAaaAaAaAaaAaAAAAAAAAaaaAaAaAaaAaaAa"), params.BeaconConfig().DefaultFeeRecipient)
+
 }
 
 func TestConfigureNetwork(t *testing.T) {

beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/ethereum/go-ethereum/common"
 	"github.com/pkg/errors"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
@@ -130,7 +131,7 @@ func (vs *Server) getExecutionPayload(ctx context.Context, slot types.Slot, vIdx
 		if bytes.Equal(feeRecipient.Bytes(), burnAddr) {
 			logrus.WithFields(logrus.Fields{
 				"validatorIndex": vIdx,
-				"burnAddress":    burnAddr,
+				"burnAddress":    common.BytesToAddress(burnAddr).Hex(),
 			}).Error("Fee recipient not set. Using burn address")
 		}
 	default:

validator/node/BUILD.bazel

@@ -11,6 +11,7 @@ go_test(
         "//config/fieldparams:go_default_library",
         "//config/validator/service:go_default_library",
         "//encoding/bytesutil:go_default_library",
+        "//testing/assert:go_default_library",
         "//testing/require:go_default_library",
         "//validator/accounts:go_default_library",
         "//validator/accounts/wallet:go_default_library",

validator/node/node.go

@@ -539,8 +539,19 @@ func feeRecipientConfig(cliCtx *cli.Context) (*validatorServiceConfig.FeeRecipie
 			if !common.IsHexAddress(option.FeeRecipient) {
 				return nil, errors.New("fee recipient is not a valid eth1 address")
 			}
+			mixedcaseAddress, err := common.NewMixedcaseAddressFromString(option.FeeRecipient)
+			if err != nil {
+				return nil, errors.Wrapf(err, "could not decode fee recipient %s", option.FeeRecipient)
+			}
+			checksumAddress := common.BytesToAddress(feebytes)
+			if !mixedcaseAddress.ValidChecksum() {
+				log.Warnf("Fee recipient %s is not a checksum Ethereum address. "+
+					"The checksummed address is %s and will be used as the fee recipient. "+
+					"We recommend using a mixed-case address (checksum) "+
+					"to prevent spelling mistakes in your fee recipient Ethereum address", option.FeeRecipient, checksumAddress.Hex())
+			}
 			frConfig.ProposeConfig[bytesutil.ToBytes48(decodedKey)] = &validatorServiceConfig.FeeRecipientOptions{
-				FeeRecipient: common.BytesToAddress(feebytes),
+				FeeRecipient: checksumAddress,
 			}
 		}
 	}

validator/node/node_test.go

@@ -16,6 +16,7 @@ import (
 	fieldparams "github.com/prysmaticlabs/prysm/config/fieldparams"
 	validator_service_config "github.com/prysmaticlabs/prysm/config/validator/service"
 	"github.com/prysmaticlabs/prysm/encoding/bytesutil"
+	"github.com/prysmaticlabs/prysm/testing/assert"
 	"github.com/prysmaticlabs/prysm/testing/require"
 	"github.com/prysmaticlabs/prysm/validator/accounts"
 	"github.com/prysmaticlabs/prysm/validator/accounts/wallet"
@@ -312,6 +313,8 @@ func TestUnmarshalFromURL(t *testing.T) {
 }
 
 func TestFeeRecipientConfig(t *testing.T) {
+	hook := logTest.NewGlobal()
+
 	type feeRecipientFlag struct {
 		dir        string
 		url        string
@@ -326,12 +329,13 @@ func TestFeeRecipientConfig(t *testing.T) {
 		want        func() *validator_service_config.FeeRecipientConfig
 		urlResponse string
 		wantErr     string
+		wantLog     string
 	}{
 		{
-			name: "Happy Path Config file File",
+			name: "Happy Path Config file File, bad checksum",
 			args: args{
 				feeRecipientFlagValues: &feeRecipientFlag{
-					dir:        "./testdata/good-prepare-beacon-proposer-config.json",
+					dir:        "./testdata/good-prepare-beacon-proposer-config-badchecksum.json",
 					url:        "",
 					defaultfee: "",
 				},
@@ -342,15 +346,16 @@ func TestFeeRecipientConfig(t *testing.T) {
 				return &validator_service_config.FeeRecipientConfig{
 					ProposeConfig: map[[fieldparams.BLSPubkeyLength]byte]*validator_service_config.FeeRecipientOptions{
 						bytesutil.ToBytes48(key1): {
-							FeeRecipient: common.HexToAddress("0x50155530FCE8a85ec7055A5F8b2bE214B3DaeFd3"),
+							FeeRecipient: common.HexToAddress("0xae967917c465db8578ca9024c205720b1a3651A9"),
 						},
 					},
 					DefaultConfig: &validator_service_config.FeeRecipientOptions{
-						FeeRecipient: common.HexToAddress("0x6e35733c5af9B61374A128e6F85f553aF09ff89A"),
+						FeeRecipient: common.HexToAddress("0xae967917c465db8578ca9024c205720b1a3651A9"),
 					},
 				}
 			},
 			wantErr: "",
+			wantLog: "is not a checksum Ethereum address",
 		},
 		{
 			name: "Happy Path Config file File multiple fee recipients",
@@ -506,6 +511,11 @@ func TestFeeRecipientConfig(t *testing.T) {
 				require.ErrorContains(t, tt.wantErr, err)
 				return
 			}
+			if tt.wantLog != "" {
+				assert.LogsContain(t, hook,
+					tt.wantLog,
+				)
+			}
 			w := tt.want()
 			require.DeepEqual(t, w, got)
 		})

validator/node/testdata/good-prepare-beacon-proposer-config-badchecksum.json

@@ -0,0 +1,10 @@
+{
+  "proposer_config": {
+    "0xa057816155ad77931185101128655c0191bd0214c201ca48ed887f6c4c6adf334070efcd75140eada5ac83a92506dd7a": {
+      "fee_recipient": "0xae967917c465db8578ca9024c205720b1a3651A9"
+    }
+  },
+  "default_config": {
+    "fee_recipient": "0xae967917c465db8578ca9024c205720b1a3651A9"
+  }
+}