diff --git a/cl/beacon/handler/block_production.go b/cl/beacon/handler/block_production.go index 89adbad8e3e..0d1a155e49d 100644 --- a/cl/beacon/handler/block_production.go +++ b/cl/beacon/handler/block_production.go @@ -227,6 +227,68 @@ func computeBlockBuilderWindow(now, slotStart time.Time, cfg *clparams.BeaconCha } } +// payloadAttributes builds the attributes for a version of the forkchoice call, which is the one +// place that decides which fields each version carries. A field the chosen version does not define +// is left unpopulated rather than filled in and ignored: V1 has no withdrawals and V1 and V2 no +// parent beacon block root, and an execution client rejects a request that supplies them. +func payloadAttributes( + version clparams.StateVersion, + timestamp hexutil.Uint64, + prevRandao common.Hash, + feeRecipient common.Address, + withdrawals []*types.Withdrawal, + parentRoot *common.Hash, + slotNumber, targetGasLimit *hexutil.Uint64, +) *engine_types.PayloadAttributes { + attrs := &engine_types.PayloadAttributes{ + Timestamp: timestamp, + PrevRandao: prevRandao, + SuggestedFeeRecipient: feeRecipient, + } + if version.AfterOrEqual(clparams.CapellaVersion) { + attrs.Withdrawals = withdrawals + } + if version.AfterOrEqual(clparams.DenebVersion) { + attrs.ParentBeaconBlockRoot = parentRoot + } + if version.AfterOrEqual(clparams.GloasVersion) { + attrs.SlotNumber = slotNumber + attrs.TargetGasLimit = targetGasLimit + } + return attrs +} + +// expectedWithdrawals resolves the withdrawals for the payload being built. Under Gloas the source +// depends on whether the head's payload was revealed: a FULL head is read from the state copy with +// that payload applied, an EMPTY one from the expectation the state already cached. +func (a *ApiHandler) expectedWithdrawals( + baseState, withParentPayload *state.CachingBeaconState, + stateVersion clparams.StateVersion, + targetSlot uint64, +) ([]*types.Withdrawal, error) { + epoch := targetSlot / a.beaconChainCfg.SlotsPerEpoch + if stateVersion.Before(clparams.GloasVersion) || withParentPayload != nil { + source := baseState + if withParentPayload != nil { + source = withParentPayload + } + clWithdrawals, err := state.GetExpectedWithdrawals(source, epoch) + if err != nil { + return nil, err + } + return cltypes.ConvertConsensusWithdrawalsToExecutionWithdrawals(clWithdrawals.Withdrawals), nil + } + cached := baseState.GetPayloadExpectedWithdrawals() + if cached == nil { + return nil, nil + } + consensusWithdrawals := make([]*cltypes.Withdrawal, cached.Len()) + for i := range consensusWithdrawals { + consensusWithdrawals[i] = cached.Get(i) + } + return cltypes.ConvertConsensusWithdrawalsToExecutionWithdrawals(consensusWithdrawals), nil +} + func shouldRetryGetPayload(now, deadline time.Time) bool { return now.Before(deadline) } @@ -976,75 +1038,22 @@ func (a *ApiHandler) produceBeaconBody( }() retryTime := 10 * time.Millisecond feeRecipient, _ := a.validatorParams.GetFeeRecipient(proposerIndex) - var withdrawals []*types.Withdrawal - switch { - case gloasWithdrawalsState != nil: - // GLOAS FULL: compute withdrawals from the state copy with parent payload applied - clWithdrawals, err := state.GetExpectedWithdrawals( - gloasWithdrawalsState, - targetSlot/a.beaconChainCfg.SlotsPerEpoch, - ) - if err != nil { - log.Error("BlockProduction: GetExpectedWithdrawals (FULL) failed", "err", err) - return - } - withdrawals = make([]*types.Withdrawal, 0, len(clWithdrawals.Withdrawals)) - for _, w := range clWithdrawals.Withdrawals { - withdrawals = append(withdrawals, &types.Withdrawal{ - Index: w.Index, - Amount: w.Amount, - Validator: w.Validator, - Address: w.Address, - }) - } - case stateVersion >= clparams.GloasVersion && gloasWithdrawalsState == nil: - // GLOAS EMPTY: use cached payload_expected_withdrawals from state - cachedWithdrawals := baseState.GetPayloadExpectedWithdrawals() - if cachedWithdrawals != nil { - withdrawals = make([]*types.Withdrawal, 0, cachedWithdrawals.Len()) - for i := 0; i < cachedWithdrawals.Len(); i++ { - w := cachedWithdrawals.Get(i) - withdrawals = append(withdrawals, &types.Withdrawal{ - Index: w.Index, - Amount: w.Amount, - Validator: w.Validator, - Address: w.Address, - }) - } - } - default: - // Pre-GLOAS: compute withdrawals normally - clWithdrawals, err := state.GetExpectedWithdrawals( - baseState, - targetSlot/a.beaconChainCfg.SlotsPerEpoch, - ) - if err != nil { - log.Error("BlockProduction: GetExpectedWithdrawals failed", "err", err) - return - } - withdrawals = make([]*types.Withdrawal, 0, len(clWithdrawals.Withdrawals)) - for _, w := range clWithdrawals.Withdrawals { - withdrawals = append(withdrawals, &types.Withdrawal{ - Index: w.Index, - Amount: w.Amount, - Validator: w.Validator, - Address: w.Address, - }) - } - } - - attrs := &engine_types.PayloadAttributes{ - Timestamp: hexutil.Uint64(state.ComputeTimestampAtSlot(baseState, targetSlot)), - PrevRandao: random, - SuggestedFeeRecipient: feeRecipient, - Withdrawals: withdrawals, - ParentBeaconBlockRoot: (*common.Hash)(&blockRoot), - } - if stateVersion.AfterOrEqual(clparams.GloasVersion) { - sn := hexutil.Uint64(targetSlot) - attrs.SlotNumber = &sn - attrs.TargetGasLimit = targetGasLimit + withdrawals, err := a.expectedWithdrawals(baseState, gloasWithdrawalsState, stateVersion, targetSlot) + if err != nil { + log.Error("BlockProduction: GetExpectedWithdrawals failed", "err", err) + return } + slotNumber := hexutil.Uint64(targetSlot) + attrs := payloadAttributes( + stateVersion, + hexutil.Uint64(state.ComputeTimestampAtSlot(baseState, targetSlot)), + random, + feeRecipient, + withdrawals, + (*common.Hash)(&blockRoot), + &slotNumber, + targetGasLimit, + ) builderStartedAt := time.Now() idBytes, err := a.engine.ForkChoiceUpdate( ctx, diff --git a/cl/beacon/handler/block_production_test.go b/cl/beacon/handler/block_production_test.go index d52bb8594c8..ddafb3b2085 100644 --- a/cl/beacon/handler/block_production_test.go +++ b/cl/beacon/handler/block_production_test.go @@ -36,6 +36,7 @@ import ( "github.com/erigontech/erigon/cl/clparams" "github.com/erigontech/erigon/cl/cltypes" "github.com/erigontech/erigon/cl/cltypes/solid" + "github.com/erigontech/erigon/cl/phase1/core/state" "github.com/erigontech/erigon/cl/phase1/execution_client" "github.com/erigontech/erigon/common" "github.com/erigontech/erigon/common/hexutil" @@ -732,3 +733,99 @@ func TestCaplinBlockProductionGlamsterdamSlotNumber(t *testing.T) { require.Equal(t, hexutil.Uint64(targetSlot), *spy.lastAttributes.SlotNumber, "SlotNumber should equal the target slot") } + +func TestExpectedWithdrawalsReadsTheRightSourcePerFork(t *testing.T) { + cfg := clparams.MainnetBeaconConfig + cfg.AltairForkEpoch, cfg.BellatrixForkEpoch, cfg.CapellaForkEpoch = 0, 0, 0 + a := &ApiHandler{beaconChainCfg: &cfg} + + capellaState := state.New(&cfg) + capellaState.SetVersion(clparams.CapellaVersion) + + // Before Gloas the expectation is computed from the head state itself, and the list is present + // even when empty: the execution layer rejects a nil one after Shanghai. + withdrawals, err := a.expectedWithdrawals(capellaState, nil, clparams.CapellaVersion, 0) + require.NoError(t, err) + require.NotNil(t, withdrawals) + require.Empty(t, withdrawals) + + gloasState := state.New(&cfg) + gloasState.SetVersion(clparams.GloasVersion) + + // A Gloas head whose payload was revealed is read from the state copy carrying that payload, + // not from the head state. Only that copy carries a pending builder withdrawal, so reading the + // wrong one comes back empty rather than merely equal. + withParentPayload := state.New(&cfg) + withParentPayload.SetVersion(clparams.GloasVersion) + pending := solid.NewDynamicListSSZ[*cltypes.BuilderPendingWithdrawal](int(cfg.MaxWithdrawalsPerPayload)) + pending.Append(&cltypes.BuilderPendingWithdrawal{FeeRecipient: common.Address{0xbb}, Amount: 12, BuilderIndex: 3}) + withParentPayload.SetBuilderPendingWithdrawals(pending) + + withdrawals, err = a.expectedWithdrawals(gloasState, withParentPayload, clparams.GloasVersion, 0) + require.NoError(t, err) + require.Equal(t, []*types.Withdrawal{{ + Index: 0, + Validator: state.ConvertBuilderIndexToValidatorIndex(3), + Address: common.Address{0xbb}, + Amount: 12, + }}, withdrawals) + + // An EMPTY Gloas head uses the expectation the state already cached rather than computing a + // fresh one, so what it returns is whatever was cached. + withdrawals, err = a.expectedWithdrawals(gloasState, nil, clparams.GloasVersion, 0) + require.NoError(t, err) + require.Empty(t, withdrawals) + + cached := solid.NewDynamicListSSZ[*cltypes.Withdrawal](int(cfg.MaxWithdrawalsPerPayload)) + cached.Append(&cltypes.Withdrawal{Index: 7, Validator: 8, Address: common.Address{0xaa}, Amount: 9}) + gloasState.SetPayloadExpectedWithdrawals(cached) + withdrawals, err = a.expectedWithdrawals(gloasState, nil, clparams.GloasVersion, 0) + require.NoError(t, err) + require.Equal(t, []*types.Withdrawal{ + {Index: 7, Validator: 8, Address: common.Address{0xaa}, Amount: 9}, + }, withdrawals) +} + +func TestPayloadAttributesOmitFieldsTheChosenVersionCannotCarry(t *testing.T) { + root := common.Hash{0xaa} + withdrawals := []*types.Withdrawal{{Index: 1}} + slotNumber := hexutil.Uint64(64) + targetGasLimit := hexutil.Uint64(36_000_000) + + for _, tc := range []struct { + version clparams.StateVersion + wantWithdrawals bool + wantParentRoot bool + wantGloasFields bool + }{ + {clparams.BellatrixVersion, false, false, false}, + {clparams.CapellaVersion, true, false, false}, + {clparams.DenebVersion, true, true, false}, + {clparams.FuluVersion, true, true, false}, + {clparams.GloasVersion, true, true, true}, + } { + t.Run(tc.version.String(), func(t *testing.T) { + attrs := payloadAttributes(tc.version, 1, common.Hash{0xbb}, common.Address{0xcc}, + withdrawals, &root, &slotNumber, &targetGasLimit) + + // A version that does not define a field must not have it populated: V1 carries no + // withdrawals, V1 and V2 no parent beacon block root, and supplying one is rejected + // rather than ignored. + require.Equal(t, tc.wantWithdrawals, attrs.Withdrawals != nil) + require.Equal(t, tc.wantParentRoot, attrs.ParentBeaconBlockRoot != nil) + + // The values have to arrive, not merely be non-nil: dropping either Gloas field leaves + // every Gloas proposal rejected with -38003. + if tc.wantGloasFields { + require.Equal(t, &slotNumber, attrs.SlotNumber) + require.Equal(t, &targetGasLimit, attrs.TargetGasLimit) + } else { + require.Nil(t, attrs.SlotNumber) + require.Nil(t, attrs.TargetGasLimit) + } + require.Equal(t, hexutil.Uint64(1), attrs.Timestamp) + require.Equal(t, common.Hash{0xbb}, attrs.PrevRandao) + require.Equal(t, common.Address{0xcc}, attrs.SuggestedFeeRecipient) + }) + } +}