-
Notifications
You must be signed in to change notification settings - Fork 1.5k
cl/phase1, execution: give the execution module a typed busy signal and the caller's context #23273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| // 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 <http://www.gnu.org/licenses/>. | ||
|
|
||
| package execution_client | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/erigontech/erigon/execution/execmodule/chainreader" | ||
| ) | ||
|
|
||
| func TestRetryAssembleBlockReturnsFirstSuccess(t *testing.T) { | ||
| calls := 0 | ||
| id, err := retryAssembleBlock(t.Context(), 3, time.Millisecond, func(context.Context) (uint64, error) { | ||
| calls++ | ||
| if calls < 3 { | ||
| return 0, chainreader.ErrExecutionBusy | ||
| } | ||
| return 7, nil | ||
| }) | ||
|
|
||
| require.NoError(t, err) | ||
| require.Equal(t, uint64(7), id) | ||
| require.Equal(t, 3, calls) | ||
| } | ||
|
|
||
| func TestRetryAssembleBlockStopsOnRejection(t *testing.T) { | ||
| rejected := errors.New("withdrawals before shanghai") | ||
| calls := 0 | ||
| _, err := retryAssembleBlock(t.Context(), 30, time.Hour, func(context.Context) (uint64, error) { | ||
| calls++ | ||
| return 0, rejected | ||
| }) | ||
|
|
||
| // Only contention settles by waiting; a rejection answers the same way however often it is | ||
| // asked, so retrying it just burns the slot. | ||
| require.ErrorIs(t, err, rejected) | ||
| require.Equal(t, 1, calls) | ||
| } | ||
|
|
||
| func TestRetryAssembleBlockGivesUpAfterAttempts(t *testing.T) { | ||
| calls := 0 | ||
| _, err := retryAssembleBlock(t.Context(), 2, time.Millisecond, func(context.Context) (uint64, error) { | ||
| calls++ | ||
| return 0, chainreader.ErrExecutionBusy | ||
| }) | ||
|
|
||
| require.ErrorIs(t, err, chainreader.ErrExecutionBusy) | ||
| require.Equal(t, 2, calls) | ||
| } | ||
|
|
||
| func TestRetryAssembleBlockStopsWhenContextIsCanceled(t *testing.T) { | ||
| ctx, cancel := context.WithCancel(t.Context()) | ||
| calls := 0 | ||
| _, err := retryAssembleBlock(ctx, 30, time.Hour, func(context.Context) (uint64, error) { | ||
| calls++ | ||
| cancel() | ||
| return 0, chainreader.ErrExecutionBusy | ||
| }) | ||
|
|
||
| require.ErrorIs(t, err, context.Canceled) | ||
| require.Equal(t, 1, calls) | ||
| } | ||
|
|
||
| func TestRetryAssembleBlockDoesNotStartWithCanceledContext(t *testing.T) { | ||
| ctx, cancel := context.WithCancel(t.Context()) | ||
| cancel() | ||
| calls := 0 | ||
| _, err := retryAssembleBlock(ctx, 30, time.Hour, func(context.Context) (uint64, error) { | ||
| calls++ | ||
| return 0, chainreader.ErrExecutionBusy | ||
| }) | ||
|
|
||
| require.ErrorIs(t, err, context.Canceled) | ||
| require.Zero(t, calls) | ||
| } | ||
|
|
||
| func TestRetryAssembleBlockRejectsNoAttempts(t *testing.T) { | ||
| _, err := retryAssembleBlock(t.Context(), 0, time.Millisecond, func(context.Context) (uint64, error) { | ||
| return 1, nil | ||
| }) | ||
| require.EqualError(t, err, "assemble block requires at least one attempt") | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -320,7 +320,7 @@ func (cc *ExecutionClientEngine) HasBlock(ctx context.Context, hash common.Hash) | |
|
|
||
| func (cc *ExecutionClientEngine) GetAssembledBlock(ctx context.Context, id []byte, version clparams.StateVersion) (*cltypes.Eth1Block, *engine_types.BlobsBundle, *typesproto.RequestsBundle, *big.Int, error) { | ||
| if cc.isLocal() { | ||
| return cc.chainRW.GetAssembledBlock(binary.LittleEndian.Uint64(id)) | ||
| return cc.chainRW.GetAssembledBlock(ctx, binary.LittleEndian.Uint64(id)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we cover this local-engine path, and the direct-client equivalent, with a canceled-context test that reaches the blocking builder fixture? The existing cancellation test stops at |
||
| } | ||
|
|
||
| // GetPayload versions advance with the response fields introduced by each fork. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add a public-path test that drives an
ExecModuleBusy result throughChainReaderWriterEth1intoForkChoiceUpdate, then verifies Busy -> success retries and Busy -> permanent error stops? The current helper tests injectErrExecutionBusydirectly, so they would still pass if the Busy-to-sentinel mapping or this wiring regressed. Non-blocking, but this is the production sequence the change is protecting.