From 249b7c2fb7d0203d0493d99f6268a8888dafb894 Mon Sep 17 00:00:00 2001 From: lystopad Date: Fri, 14 Aug 2026 10:55:13 +0000 Subject: [PATCH] cl: reuse one converter between consensus and execution withdrawals (#23271) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split out of #23105, which grew too large to review in one piece. This is the mechanical, behaviour-neutral part. The conversion between `cltypes.Withdrawal` and `types.Withdrawal` was hand-written at each call site. `cl/cltypes/withdrawal.go` already had a private singular converter, so this exports a plural one and uses it in the two places that built the list by hand: the payload-attributes emitter in the forkchoice stage, and `cacheExecutionBody`. No behaviour change. Both sites produce the same slice as before, including whether it comes back nil — `cacheExecutionBody` keeps returning nil for an empty withdrawals list rather than an empty slice. Part of a series splitting #23105 into reviewable units. The remaining parts follow separately; this one stands alone and depends on nothing else in the series. (cherry picked from commit 67a305dbbee30d9d7e5d3c172030f5dc1a2ea858) --- cl/beacon/handler/block_production.go | 13 +++---- cl/cltypes/withdrawal.go | 11 ++++++ cl/cltypes/withdrawal_test.go | 56 +++++++++++++++++++++++++++ cl/phase1/stages/forkchoice.go | 12 +----- 4 files changed, 74 insertions(+), 18 deletions(-) create mode 100644 cl/cltypes/withdrawal_test.go diff --git a/cl/beacon/handler/block_production.go b/cl/beacon/handler/block_production.go index a7814692e76..e33f2017658 100644 --- a/cl/beacon/handler/block_production.go +++ b/cl/beacon/handler/block_production.go @@ -2572,16 +2572,13 @@ func (a *ApiHandler) cacheExecutionBody(payload *cltypes.Eth1Block) { }) } var ws []*types.Withdrawal - if payload.Withdrawals != nil { - payload.Withdrawals.Range(func(idx int, w *cltypes.Withdrawal, total int) bool { - ws = append(ws, &types.Withdrawal{ - Index: w.Index, - Validator: w.Validator, - Address: w.Address, - Amount: w.Amount, - }) + if payload.Withdrawals != nil && payload.Withdrawals.Len() > 0 { + consensusWithdrawals := make([]*cltypes.Withdrawal, payload.Withdrawals.Len()) + payload.Withdrawals.Range(func(idx int, w *cltypes.Withdrawal, _ int) bool { + consensusWithdrawals[idx] = w return true }) + ws = cltypes.ConvertConsensusWithdrawalsToExecutionWithdrawals(consensusWithdrawals) } a.blockReader.CacheBlockBody(payload.BlockNumber, rawTxs, ws) } diff --git a/cl/cltypes/withdrawal.go b/cl/cltypes/withdrawal.go index 40447af3601..aae5abb6f4a 100644 --- a/cl/cltypes/withdrawal.go +++ b/cl/cltypes/withdrawal.go @@ -87,6 +87,17 @@ func convertExecutionWithdrawalsToConsensusWithdrawals(executionWithdrawal []*ty return ret } +// ConvertConsensusWithdrawalsToExecutionWithdrawals converts a withdrawal list to its execution +// representation, in order and with no shared pointers. The result is never nil, which matters +// because the execution layer rejects a nil list and an empty one under opposite conditions. +func ConvertConsensusWithdrawalsToExecutionWithdrawals(consensusWithdrawals []*Withdrawal) []*types.Withdrawal { + ret := make([]*types.Withdrawal, len(consensusWithdrawals)) + for i, w := range consensusWithdrawals { + ret[i] = convertConsensusWithdrawalToExecutionWithdrawal(w) + } + return ret +} + // ExpectedWithdrawals represents the expected withdrawals for a beacon state type ExpectedWithdrawals struct { Withdrawals []*Withdrawal `json:"withdrawals"` diff --git a/cl/cltypes/withdrawal_test.go b/cl/cltypes/withdrawal_test.go new file mode 100644 index 00000000000..de883a43058 --- /dev/null +++ b/cl/cltypes/withdrawal_test.go @@ -0,0 +1,56 @@ +// Copyright 2026 The Erigon Authors +// This file is part of Erigon. +// +// Erigon is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Erigon is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with Erigon. If not, see . + +package cltypes + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/erigontech/erigon/common" + "github.com/erigontech/erigon/execution/types" +) + +func TestConvertConsensusWithdrawalsToExecutionWithdrawals(t *testing.T) { + t.Parallel() + + source := []*Withdrawal{ + {Index: 1, Validator: 10, Address: common.Address{0xaa}, Amount: 100}, + {Index: 2, Validator: 20, Address: common.Address{0xbb}, Amount: 200}, + } + + converted := ConvertConsensusWithdrawalsToExecutionWithdrawals(source) + + require.Equal(t, []*types.Withdrawal{ + {Index: 1, Validator: 10, Address: common.Address{0xaa}, Amount: 100}, + {Index: 2, Validator: 20, Address: common.Address{0xbb}, Amount: 200}, + }, converted) + + // The execution layer owns its copy: mutating the source afterwards must not reach it. + source[0].Amount = 999 + require.Equal(t, uint64(100), converted[0].Amount) +} + +func TestConvertConsensusWithdrawalsToExecutionWithdrawalsNeverReturnsNil(t *testing.T) { + t.Parallel() + + // A nil list and an empty one are rejected by the execution layer under opposite conditions, + // so an absent input must not become an absent list. + require.NotNil(t, ConvertConsensusWithdrawalsToExecutionWithdrawals(nil)) + require.Empty(t, ConvertConsensusWithdrawalsToExecutionWithdrawals(nil)) + require.NotNil(t, ConvertConsensusWithdrawalsToExecutionWithdrawals([]*Withdrawal{})) +} diff --git a/cl/phase1/stages/forkchoice.go b/cl/phase1/stages/forkchoice.go index e97dc5cb590..ec39d95f01f 100644 --- a/cl/phase1/stages/forkchoice.go +++ b/cl/phase1/stages/forkchoice.go @@ -13,6 +13,7 @@ import ( "github.com/erigontech/erigon/cl/beacon/beaconevents" "github.com/erigontech/erigon/cl/beacon/synced_data" "github.com/erigontech/erigon/cl/clparams" + "github.com/erigontech/erigon/cl/cltypes" "github.com/erigontech/erigon/cl/monitor" "github.com/erigontech/erigon/cl/monitor/shuffling_metrics" "github.com/erigontech/erigon/cl/persistence/beacon_indicies" @@ -26,7 +27,6 @@ import ( "github.com/erigontech/erigon/common/log/v3" "github.com/erigontech/erigon/db/kv" "github.com/erigontech/erigon/execution/engineapi/engine_types" - "github.com/erigontech/erigon/execution/types" ) // computeAndNotifyServicesOfNewForkChoice calculates the new head of the fork choice and notifies relevant services. @@ -247,19 +247,11 @@ func emitNextPaylodAttributesEvent(cfg *Cfg, headSlot uint64, headRoot common.Ha log.Warn("failed to get proposer index", "err", err) return err } - withdrawals := []*types.Withdrawal{} expWithdrawals, err := state.GetExpectedWithdrawals(s, epoch) if err != nil { return err } - for _, w := range expWithdrawals.Withdrawals { - withdrawals = append(withdrawals, &types.Withdrawal{ - Amount: w.Amount, - Index: w.Index, - Validator: w.Validator, - Address: w.Address, - }) - } + withdrawals := cltypes.ConvertConsensusWithdrawalsToExecutionWithdrawals(expWithdrawals.Withdrawals) payloadAttributes := engine_types.PayloadAttributes{ Timestamp: hexutil.Uint64(headPayloadHeader.Time + cfg.beaconCfg.SecondsPerSlot), PrevRandao: randaoMix,