*: check fee recipient on local (#4474)

* Fix warning for mismatching fee recipient to be on blinded block rather than PBS enabled

* Add metrics and healthchecks

* Fix metrics.md

Author: KaloyanTanev

app/health/checks.go

@@ -261,6 +261,26 @@ var checks = []check{
 			return maxVal == 2, nil // 1=blinded (MEV), 2=local block
 		},
 	},
+	{
+		Name: "local_proposal_fee_recipient_mismatch",
+		Description: `Local block proposal has a mismatched fee recipient.
+		The fee recipient in the fetched local proposal does not match the expected fee recipient configured for the validator.
+		This means block rewards are highly likely to be sent to the wrong address.
+
+		This usually happens in a rare scenario where Charon has multiple BNs set and a subset of them fail the prepare proposer call (https://ethereum.github.io/beacon-APIs/#/ValidatorRequiredApi/prepareBeaconProposer),
+		but another subsest succeed. Then when the proposal is fetched, it is fetched from the BN that failed the prepare proposer call, which returns a local block with an unexpected fee recipient (likely the zero burn address).
+
+		At this stage there is not straightforward fix for that. What an operator can do is to identify the faulty BN (by checking which one has failed prepare proposer calls in its logs) and remove it from the subset of BNs.`,
+		Severity: severityCritical,
+		Func: func(q query, _ Metadata) (bool, error) {
+			maxVal, err := q("core_fetcher_proposal_local_mismatch_fee_recipient", noLabels, gaugeMax)
+			if err != nil {
+				return false, err
+			}
+
+			return maxVal == 2.0, nil // 0.0=N/A, 1.0=match, 2.0=mismatch
+		},
+	},
 	{
 		Name: "high_beacon_node_sse_head_delay",
 		Description: `Beacon node SSE head received after 4s for >4% of the blocks in the past hour.

app/health/checks_internal_test.go

@@ -457,6 +457,28 @@ func TestLocalBlockProposalCheck(t *testing.T) {
 	})
 }
 
+func TestLocalProposalFeeRecipientMismatchCheck(t *testing.T) {
+	m := Metadata{}
+	checkName := "local_proposal_fee_recipient_mismatch"
+	metricName := "core_fetcher_proposal_local_mismatch_fee_recipient"
+
+	t.Run("no data", func(t *testing.T) {
+		testCheck(t, m, checkName, false, nil)
+	})
+
+	t.Run("fee recipient matches", func(t *testing.T) {
+		testCheck(t, m, checkName, false,
+			genFam(metricName, genGauge(nil, 1, 1, 1)),
+		)
+	})
+
+	t.Run("fee recipient mismatch", func(t *testing.T) {
+		testCheck(t, m, checkName, true,
+			genFam(metricName, genGauge(nil, 2, 2, 2)),
+		)
+	})
+}
+
 func TestHighParsigdbStoreLatencyCheck(t *testing.T) {
 	m := Metadata{}
 	checkName := "high_parsigdb_store_latency"

core/fetcher/fetcher.go

@@ -353,7 +353,7 @@ func (f *Fetcher) fetchProposerData(ctx context.Context, slot uint64, defSet cor
 		proposal := eth2Resp.Data
 
 		// Builders set fee recipient to themselves so it's always different from validator's.
-		if !f.builderEnabled {
+		if !proposal.Blinded {
 			// Ensure fee recipient is correctly populated in proposal.
 			verifyFeeRecipient(ctx, proposal, f.feeRecipientFunc(pubkey))
 		}
@@ -456,42 +456,25 @@ func verifyFeeRecipient(ctx context.Context, proposal *eth2api.VersionedProposal
 
 	switch proposal.Version {
 	case eth2spec.DataVersionBellatrix:
-		if proposal.Blinded {
-			actualAddr = fmt.Sprintf("%#x", proposal.BellatrixBlinded.Body.ExecutionPayloadHeader.FeeRecipient)
-		} else {
-			actualAddr = fmt.Sprintf("%#x", proposal.Bellatrix.Body.ExecutionPayload.FeeRecipient)
-		}
+		actualAddr = fmt.Sprintf("%#x", proposal.Bellatrix.Body.ExecutionPayload.FeeRecipient)
 	case eth2spec.DataVersionCapella:
-		if proposal.Blinded {
-			actualAddr = fmt.Sprintf("%#x", proposal.CapellaBlinded.Body.ExecutionPayloadHeader.FeeRecipient)
-		} else {
-			actualAddr = fmt.Sprintf("%#x", proposal.Capella.Body.ExecutionPayload.FeeRecipient)
-		}
+		actualAddr = fmt.Sprintf("%#x", proposal.Capella.Body.ExecutionPayload.FeeRecipient)
 	case eth2spec.DataVersionDeneb:
-		if proposal.Blinded {
-			actualAddr = fmt.Sprintf("%#x", proposal.DenebBlinded.Body.ExecutionPayloadHeader.FeeRecipient)
-		} else {
-			actualAddr = fmt.Sprintf("%#x", proposal.Deneb.Block.Body.ExecutionPayload.FeeRecipient)
-		}
+		actualAddr = fmt.Sprintf("%#x", proposal.Deneb.Block.Body.ExecutionPayload.FeeRecipient)
 	case eth2spec.DataVersionElectra:
-		if proposal.Blinded {
-			actualAddr = fmt.Sprintf("%#x", proposal.ElectraBlinded.Body.ExecutionPayloadHeader.FeeRecipient)
-		} else {
-			actualAddr = fmt.Sprintf("%#x", proposal.Electra.Block.Body.ExecutionPayload.FeeRecipient)
-		}
+		actualAddr = fmt.Sprintf("%#x", proposal.Electra.Block.Body.ExecutionPayload.FeeRecipient)
 	case eth2spec.DataVersionFulu:
-		if proposal.Blinded {
-			actualAddr = fmt.Sprintf("%#x", proposal.FuluBlinded.Body.ExecutionPayloadHeader.FeeRecipient)
-		} else {
-			actualAddr = fmt.Sprintf("%#x", proposal.Fulu.Block.Body.ExecutionPayload.FeeRecipient)
-		}
+		actualAddr = fmt.Sprintf("%#x", proposal.Fulu.Block.Body.ExecutionPayload.FeeRecipient)
 	default:
 		return
 	}
 
 	if actualAddr != "" && !strings.EqualFold(actualAddr, feeRecipientAddress) {
 		log.Warn(ctx, "Proposal with unexpected fee recipient address", nil,
 			z.Str("expected", feeRecipientAddress), z.Str("actual", actualAddr))
+		proposalLocalMismatchFeeRecipientGauge.Set(2.0)
+	} else {
+		proposalLocalMismatchFeeRecipientGauge.Set(1.0)
 	}
 }
 

core/fetcher/fetcher_internal_test.go

@@ -26,77 +26,42 @@ func TestVerifyFeeRecipient(t *testing.T) {
 			name: "bellatrix",
 			proposal: eth2api.VersionedProposal{
 				Version:   eth2spec.DataVersionBellatrix,
+				Blinded:   false,
 				Bellatrix: testutil.RandomBellatrixBeaconBlock(),
 			},
 		},
-		{
-			name: "bellatrix blinded",
-			proposal: eth2api.VersionedProposal{
-				Version:          eth2spec.DataVersionBellatrix,
-				BellatrixBlinded: testutil.RandomBellatrixBlindedBeaconBlock(),
-				Blinded:          true,
-			},
-		},
 		{
 			name: "capella",
 			proposal: eth2api.VersionedProposal{
 				Version: eth2spec.DataVersionCapella,
+				Blinded: false,
 				Capella: testutil.RandomCapellaBeaconBlock(),
 			},
 		},
-		{
-			name: "capella blinded",
-			proposal: eth2api.VersionedProposal{
-				Version:        eth2spec.DataVersionCapella,
-				CapellaBlinded: testutil.RandomCapellaBlindedBeaconBlock(),
-				Blinded:        true,
-			},
-		},
 		{
 			name: "deneb",
 			proposal: eth2api.VersionedProposal{
 				Version: eth2spec.DataVersionDeneb,
+				Blinded: false,
 				Deneb:   testutil.RandomDenebVersionedProposal().Deneb,
 			},
 		},
-		{
-			name: "deneb blinded",
-			proposal: eth2api.VersionedProposal{
-				Version:      eth2spec.DataVersionDeneb,
-				DenebBlinded: testutil.RandomDenebBlindedBeaconBlock(),
-				Blinded:      true,
-			},
-		},
 		{
 			name: "electra",
 			proposal: eth2api.VersionedProposal{
 				Version: eth2spec.DataVersionElectra,
+				Blinded: false,
 				Electra: testutil.RandomElectraVersionedProposal().Electra,
 			},
 		},
-		{
-			name: "electra blinded",
-			proposal: eth2api.VersionedProposal{
-				Version:        eth2spec.DataVersionElectra,
-				ElectraBlinded: testutil.RandomElectraBlindedBeaconBlock(),
-				Blinded:        true,
-			},
-		},
 		{
 			name: "fulu",
 			proposal: eth2api.VersionedProposal{
 				Version: eth2spec.DataVersionFulu,
+				Blinded: false,
 				Fulu:    testutil.RandomFuluVersionedProposal().Fulu,
 			},
 		},
-		{
-			name: "fulu blinded",
-			proposal: eth2api.VersionedProposal{
-				Version:     eth2spec.DataVersionFulu,
-				FuluBlinded: testutil.RandomElectraBlindedBeaconBlock(),
-				Blinded:     true,
-			},
-		},
 	}
 
 	for _, test := range tests {

core/fetcher/metrics.go

@@ -14,3 +14,10 @@ var proposalBlindedGauge = promauto.NewGauge(prometheus.GaugeOpts{
 	Name:      "proposal_blinded",
 	Help:      "Whether the fetched proposal was blinded (1) or local (2)",
 })
+
+var proposalLocalMismatchFeeRecipientGauge = promauto.NewGauge(prometheus.GaugeOpts{
+	Namespace: "core",
+	Subsystem: "fetcher",
+	Name:      "proposal_local_mismatch_fee_recipient",
+	Help:      "Counts the number of times a local proposal has a mismatched fee recipient",
+})

docs/metrics.md

@@ -62,6 +62,7 @@ when storing metrics from multiple nodes or clusters in one Prometheus instance.
 | `core_consensus_error_total` | Counter | Total count of consensus errors by protocol | `protocol` |
 | `core_consensus_timeout_total` | Counter | Total count of consensus timeouts by protocol, duty, and timer | `protocol, duty, timer` |
 | `core_fetcher_proposal_blinded` | Gauge | Whether the fetched proposal was blinded (1) or local (2) |  |
+| `core_fetcher_proposal_local_mismatch_fee_recipient` | Gauge | Counts the number of times a local proposal has a mismatched fee recipient |  |
 | `core_parsigdb_exit_total` | Counter | Total number of partially signed voluntary exits per public key | `pubkey` |
 | `core_parsigdb_store` | Histogram | Latency of partial signatures received since earliest expected time, per duty, per peer index | `duty, peer_idx` |
 | `core_parsigex_set_verification_seconds` | Histogram | Duration to verify all partial signatures in a received set, in seconds | `duty` |