diff --git a/app/health/checks.go b/app/health/checks.go index 16d52dded1..802e7aadf8 100644 --- a/app/health/checks.go +++ b/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. diff --git a/app/health/checks_internal_test.go b/app/health/checks_internal_test.go index fb53502637..46fb12653e 100644 --- a/app/health/checks_internal_test.go +++ b/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" diff --git a/core/fetcher/fetcher.go b/core/fetcher/fetcher.go index 5a7f2222c0..97a31686bc 100644 --- a/core/fetcher/fetcher.go +++ b/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,35 +456,15 @@ 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 } @@ -492,6 +472,9 @@ func verifyFeeRecipient(ctx context.Context, proposal *eth2api.VersionedProposal 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) } } diff --git a/core/fetcher/fetcher_internal_test.go b/core/fetcher/fetcher_internal_test.go index 5df88969d6..54f052fea9 100644 --- a/core/fetcher/fetcher_internal_test.go +++ b/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 { diff --git a/core/fetcher/metrics.go b/core/fetcher/metrics.go index ea7286e5e5..ca01ee4aa6 100644 --- a/core/fetcher/metrics.go +++ b/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", +}) diff --git a/docs/metrics.md b/docs/metrics.md index cdb3969ad9..f9ec5999c1 100644 --- a/docs/metrics.md +++ b/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` |