From 59041cf8687b6b3c3ff43c5de3794d7451d7bb0c Mon Sep 17 00:00:00 2001
From: Raul Jordan <raul@prysmaticlabs.com>
Date: Tue, 3 May 2022 15:18:42 +0000
Subject: [PATCH] Warn Users if EE Returns Unexpected Fee Recipient Address
 (#10604)

Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com>
---
 .../validator/proposer_execution_payload.go   | 15 ++++-
 .../proposer_execution_payload_test.go        | 64 +++++++++++++++++++
 2 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go
index 313c119c5..8c25adaaf 100644
--- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go
+++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload.go
@@ -3,6 +3,7 @@ package validator
 import (
 	"bytes"
 	"context"
+	"fmt"
 
 	"github.com/pkg/errors"
 	"github.com/prometheus/client_golang/prometheus"
@@ -147,7 +148,19 @@ func (vs *Server) getExecutionPayload(ctx context.Context, slot types.Slot, vIdx
 	if payloadID == nil {
 		return nil, errors.New("nil payload id")
 	}
-	return vs.ExecutionEngineCaller.GetPayload(ctx, *payloadID)
+	payload, err := vs.ExecutionEngineCaller.GetPayload(ctx, *payloadID)
+	if err != nil {
+		return nil, err
+	}
+	// Warn if the fee recipient is not the value we expect.
+	if payload != nil && !bytes.Equal(payload.FeeRecipient, feeRecipient[:]) {
+		logrus.WithFields(logrus.Fields{
+			"wantedFeeRecipient": fmt.Sprintf("%#x", feeRecipient),
+			"received":           fmt.Sprintf("%#x", payload.FeeRecipient),
+		}).Warn("Fee recipient address from execution client is not what was expected. " +
+			"It is possible someone has compromised your client to try and take your transaction fees")
+	}
+	return payload, nil
 }
 
 // This returns the valid terminal block hash with an existence bool value.
diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload_test.go
index f2a0e1571..c6b00388d 100644
--- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload_test.go
+++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_execution_payload_test.go
@@ -19,6 +19,7 @@ import (
 	ethpb "github.com/prysmaticlabs/prysm/proto/prysm/v1alpha1"
 	"github.com/prysmaticlabs/prysm/testing/require"
 	"github.com/prysmaticlabs/prysm/testing/util"
+	logTest "github.com/sirupsen/logrus/hooks/test"
 )
 
 func TestServer_activationEpochNotReached(t *testing.T) {
@@ -133,6 +134,69 @@ func TestServer_getExecutionPayload(t *testing.T) {
 	}
 }
 
+func TestServer_getExecutionPayload_UnexpectedFeeRecipient(t *testing.T) {
+	hook := logTest.NewGlobal()
+	nonTransitionSt, _ := util.DeterministicGenesisStateBellatrix(t, 1)
+	b1pb := util.NewBeaconBlock()
+	b1r, err := b1pb.Block.HashTreeRoot()
+	require.NoError(t, err)
+	b1, err := wrapper.WrappedSignedBeaconBlock(b1pb)
+	require.NoError(t, err)
+	require.NoError(t, nonTransitionSt.SetFinalizedCheckpoint(&ethpb.Checkpoint{
+		Root: b1r[:],
+	}))
+
+	transitionSt, _ := util.DeterministicGenesisStateBellatrix(t, 1)
+	require.NoError(t, transitionSt.SetLatestExecutionPayloadHeader(&ethpb.ExecutionPayloadHeader{BlockNumber: 1}))
+	b2pb := util.NewBeaconBlockBellatrix()
+	b2r, err := b2pb.Block.HashTreeRoot()
+	require.NoError(t, err)
+	b2, err := wrapper.WrappedSignedBeaconBlock(b2pb)
+	require.NoError(t, err)
+	require.NoError(t, transitionSt.SetFinalizedCheckpoint(&ethpb.Checkpoint{
+		Root: b2r[:],
+	}))
+
+	beaconDB := dbTest.SetupDB(t)
+	require.NoError(t, beaconDB.SaveBlock(context.Background(), b1))
+	require.NoError(t, beaconDB.SaveBlock(context.Background(), b2))
+	feeRecipient := common.BytesToAddress([]byte("a"))
+	require.NoError(t, beaconDB.SaveFeeRecipientsByValidatorIDs(context.Background(), []types.ValidatorIndex{0}, []common.Address{
+		feeRecipient,
+	}))
+
+	payloadID := &pb.PayloadIDBytes{0x1}
+	payload := emptyPayload()
+	payload.FeeRecipient = feeRecipient[:]
+	vs := &Server{
+		ExecutionEngineCaller: &powtesting.EngineClient{
+			PayloadIDBytes:   payloadID,
+			ExecutionPayload: payload,
+		},
+		HeadFetcher:            &chainMock.ChainService{State: transitionSt},
+		BeaconDB:               beaconDB,
+		ProposerSlotIndexCache: cache.NewProposerPayloadIDsCache(),
+	}
+	gotPayload, err := vs.getExecutionPayload(context.Background(), transitionSt.Slot(), 0)
+	require.NoError(t, err)
+	require.NotNil(t, gotPayload)
+
+	// We should NOT be getting the warning.
+	require.LogsDoNotContain(t, hook, "Fee recipient address from execution client is not what was expected")
+	hook.Reset()
+
+	evilRecipientAddress := common.BytesToAddress([]byte("evil"))
+	payload.FeeRecipient = evilRecipientAddress[:]
+	vs.ProposerSlotIndexCache = cache.NewProposerPayloadIDsCache()
+
+	gotPayload, err = vs.getExecutionPayload(context.Background(), transitionSt.Slot(), 0)
+	require.NoError(t, err)
+	require.NotNil(t, gotPayload)
+
+	// Users should be warned.
+	require.LogsContain(t, hook, "Fee recipient address from execution client is not what was expected")
+}
+
 func TestServer_getTerminalBlockHashIfExists(t *testing.T) {
 	tests := []struct {
 		name                  string