Skip to content

Commit 67ece3f

Browse files
taratorioAskAlexSharov
taratorio
and
AskAlexSharov
authored
execution/execmodule: allow reorgs on canonical chain up to finalised hash (#20825)
closes #20888 implements ethereum/execution-apis#786 which is needed for glamsterdam-devnet-0 --------- Co-authored-by: Alex Sharov <AskAlexSharov@gmail.com>
1 parent 3761dc3 commit 67ece3f

8 files changed

Lines changed: 132 additions & 8 deletions

File tree

‎execution/engineapi/engine_api_reorg_test.go‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,3 +287,91 @@ func TestNewPayloadShouldReturnValidWhenSideChainGoingBackIsLtMaxReorgDepth(t *t
287287
require.Equal(t, enginetypes.ValidStatus, ps.Status)
288288
})
289289
}
290+
291+
func TestFcuAllowsReorgBackOnCanonicalChainWhenAfterFinalisedHash(t *testing.T) {
292+
// as per spec update: https://github.com/ethereum/execution-apis/pull/786
293+
eat := engineapitester.DefaultEngineApiTester(t)
294+
eat.Run(t, func(ctx context.Context, t *testing.T, eat engineapitester.EngineApiTester) {
295+
// deploy changer at b2
296+
transactOpts, err := bind.NewKeyedTransactorWithChainID(eat.CoinbaseKey, eat.ChainId())
297+
require.NoError(t, err)
298+
transactOpts.GasLimit = params.MaxTxnGasLimit
299+
_, txn, changer, err := contracts.DeployChanger(transactOpts, eat.ContractBackend)
300+
require.NoError(t, err)
301+
b2Canon, err := eat.MockCl.BuildCanonicalBlock(ctx)
302+
require.NoError(t, err)
303+
err = eat.TxnInclusionVerifier.VerifyTxnsInclusion(ctx, b2Canon.ExecutionPayload, txn.Hash())
304+
require.NoError(t, err)
305+
// change changer at b3
306+
txn, err = changer.Change(transactOpts)
307+
require.NoError(t, err)
308+
b3Canon, err := eat.MockCl.BuildCanonicalBlock(ctx)
309+
require.NoError(t, err)
310+
err = eat.TxnInclusionVerifier.VerifyTxnsInclusion(ctx, b3Canon.ExecutionPayload, txn.Hash())
311+
require.NoError(t, err)
312+
blockNum, err := eat.RpcApiClient.BlockNumber()
313+
require.NoError(t, err)
314+
require.Equal(t, uint64(3), blockNum)
315+
// unwind back to b2
316+
err = eat.MockCl.UpdateForkChoice(ctx, b2Canon)
317+
require.NoError(t, err)
318+
// verify canonical head went back
319+
blockNum, err = eat.RpcApiClient.BlockNumber()
320+
require.NoError(t, err)
321+
require.Equal(t, uint64(2), blockNum)
322+
})
323+
}
324+
325+
func TestFcuReturnsReorgTooDeepCode38006(t *testing.T) {
326+
// as per spec update: https://github.com/ethereum/execution-apis/pull/786
327+
genesis, coinbaseKey := engineapitester.DefaultEngineApiTesterGenesis(t)
328+
eat := engineapitester.InitialiseEngineApiTester(t, engineapitester.EngineApiTesterInitArgs{
329+
Logger: testlog.Logger(t, log.LvlDebug),
330+
DataDir: t.TempDir(),
331+
Genesis: genesis,
332+
CoinbaseKey: coinbaseKey,
333+
EthConfigTweaker: func(config *ethconfig.Config) {
334+
config.MaxReorgDepth = 2
335+
},
336+
})
337+
eat.Run(t, func(ctx context.Context, t *testing.T, eat engineapitester.EngineApiTester) {
338+
// deploy changer at b2
339+
transactOpts, err := bind.NewKeyedTransactorWithChainID(eat.CoinbaseKey, eat.ChainId())
340+
require.NoError(t, err)
341+
transactOpts.GasLimit = params.MaxTxnGasLimit
342+
_, txn, changer, err := contracts.DeployChanger(transactOpts, eat.ContractBackend)
343+
require.NoError(t, err)
344+
b2Canon, err := eat.MockCl.BuildCanonicalBlock(ctx)
345+
require.NoError(t, err)
346+
err = eat.TxnInclusionVerifier.VerifyTxnsInclusion(ctx, b2Canon.ExecutionPayload, txn.Hash())
347+
require.NoError(t, err)
348+
// change changer at b3
349+
txn, err = changer.Change(transactOpts)
350+
require.NoError(t, err)
351+
b3Canon, err := eat.MockCl.BuildCanonicalBlock(ctx)
352+
require.NoError(t, err)
353+
err = eat.TxnInclusionVerifier.VerifyTxnsInclusion(ctx, b3Canon.ExecutionPayload, txn.Hash())
354+
require.NoError(t, err)
355+
// change changer at b4
356+
txn, err = changer.Change(transactOpts)
357+
require.NoError(t, err)
358+
b4Canon, err := eat.MockCl.BuildCanonicalBlock(ctx)
359+
require.NoError(t, err)
360+
err = eat.TxnInclusionVerifier.VerifyTxnsInclusion(ctx, b4Canon.ExecutionPayload, txn.Hash())
361+
require.NoError(t, err)
362+
// change changer at b5
363+
txn, err = changer.Change(transactOpts)
364+
require.NoError(t, err)
365+
b5Canon, err := eat.MockCl.BuildCanonicalBlock(ctx)
366+
require.NoError(t, err)
367+
err = eat.TxnInclusionVerifier.VerifyTxnsInclusion(ctx, b5Canon.ExecutionPayload, txn.Hash())
368+
require.NoError(t, err)
369+
// unwind back to b2 should return "Too deep reorg" error with code 38006
370+
err = eat.MockCl.UpdateForkChoice(ctx, b2Canon)
371+
require.Error(t, err)
372+
var rpcErr rpc.Error
373+
require.ErrorAs(t, err, &rpcErr)
374+
require.Equal(t, -38006, rpcErr.ErrorCode())
375+
require.Equal(t, "Too deep reorg", rpcErr.Error())
376+
})
377+
}

‎execution/engineapi/engine_helpers/constants.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ var UnknownPayloadErr = rpc.CustomError{Code: -38001, Message: "Unknown payload"
2424
var InvalidForkchoiceStateErr = rpc.CustomError{Code: -38002, Message: "Invalid forkchoice state"}
2525
var InvalidPayloadAttributesErr = rpc.CustomError{Code: -38003, Message: "Invalid payload attributes"}
2626
var TooLargeRequestErr = rpc.CustomError{Code: -38004, Message: "Too large request"}
27+
var ReorgTooDeepErr = rpc.CustomError{Code: -38006, Message: "Too deep reorg"}
2728

2829
const PectraBanner = `
2930
'########::'########::'######::'########:'########:::::'###::::

‎execution/engineapi/engine_server.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1152,6 +1152,9 @@ func (e *EngineServer) HandleForkChoice(
11521152
if status == execmodule.ExecutionStatusInvalidForkchoice {
11531153
return nil, &engine_helpers.InvalidForkchoiceStateErr
11541154
}
1155+
if status == execmodule.ExecutionStatusReorgTooDeep {
1156+
return nil, &engine_helpers.ReorgTooDeepErr
1157+
}
11551158
if status == execmodule.ExecutionStatusBusy {
11561159
return &engine_types.PayloadStatus{Status: engine_types.SyncingStatus}, nil
11571160
}

‎execution/engineapi/engineapitester/engine_api_tester.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,8 +249,8 @@ func InitialiseEngineApiTester(t testing.TB, args EngineApiTesterInitArgs) Engin
249249
if !args.NoEmptyBlock1 {
250250
// build 1 empty block before proceeding to properly initialise everything
251251
_, err = mockCl.BuildCanonicalBlock(ctx)
252+
require.NoError(t, err)
252253
}
253-
require.NoError(t, err)
254254
return EngineApiTester{
255255
GenesisBlock: genesisBlock,
256256
CoinbaseKey: args.CoinbaseKey,

‎execution/execmodule/forkchoice.go‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -291,8 +291,29 @@ func (e *ExecModule) updateForkChoice(ctx context.Context, originalBlockHash, sa
291291
}
292292

293293
if fcuHeader.Number.Sign() > 0 {
294-
if canonicalHash == blockHash && fcuHeader.Number.Uint64() >= finishProgressBefore {
295-
// if block hash is part of the canonical chain and execution is not ahead, treat as no-op.
294+
var finalisedBlockNum uint64
295+
if finalizedHash == (common.Hash{}) {
296+
// The CL has not finalised anything yet (e.g. fresh devnet before finality kicks in).
297+
finalisedBlockNum = finishProgressBefore
298+
} else {
299+
bn, err := e.blockReader.HeaderNumber(ctx, tx, finalizedHash)
300+
if err != nil {
301+
return sendForkchoiceErrorWithoutWaiting(e.logger, outcomeCh, err, false)
302+
}
303+
if bn == nil {
304+
return sendForkchoiceResultWithoutWaiting(outcomeCh, ForkChoiceResult{
305+
LatestValidHash: common.Hash{},
306+
Status: ExecutionStatusInvalidForkchoice,
307+
}, false)
308+
}
309+
finalisedBlockNum = *bn
310+
}
311+
// as per https://github.com/ethereum/execution-apis/pull/786
312+
// we short circuit reorgs if:
313+
// 1. the head is an ancestor of the last finalised block
314+
// 2. the head is a duplicate FCU (e.g. CLs sending the same FCU repeatedly)
315+
if canonicalHash == blockHash &&
316+
(fcuHeader.Number.Uint64() < finalisedBlockNum || fcuHeader.Number.Uint64() == finishProgressBefore) {
296317
writeForkChoiceHashes(tx, blockHash, safeHash, finalizedHash)
297318
valid, err := e.verifyForkchoiceHashes(ctx, tx, blockHash, finalizedHash, safeHash)
298319
if err != nil {
@@ -356,8 +377,11 @@ func (e *ExecModule) updateForkChoice(ctx context.Context, originalBlockHash, sa
356377
return sendForkchoiceErrorWithoutWaiting(e.logger, outcomeCh, err, false)
357378
}
358379
if unwindTarget < minUnwindableBlock {
359-
e.logger.Info("Reorg requested too low, capping to the minimum unwindable block", "unwindTarget", unwindTarget, "minUnwindableBlock", minUnwindableBlock)
360-
unwindTarget = minUnwindableBlock
380+
e.logger.Warn("reorg target below minimum unwindable block", "unwindTarget", unwindTarget, "minUnwindableBlock", minUnwindableBlock)
381+
return sendForkchoiceResultWithoutWaiting(outcomeCh, ForkChoiceResult{
382+
LatestValidHash: common.Hash{},
383+
Status: ExecutionStatusReorgTooDeep,
384+
}, false)
361385
}
362386

363387
if err := e.pipelineExecutor.UnwindTo(unwindTarget, stagedsync.ForkChoice, tx); err != nil {

‎execution/execmodule/interface.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ const (
3939
ExecutionStatusMissingSegment ExecutionStatus = 3
4040
ExecutionStatusInvalidForkchoice ExecutionStatus = 4
4141
ExecutionStatusBusy ExecutionStatus = 5
42+
ExecutionStatusReorgTooDeep ExecutionStatus = 6
4243
)
4344

4445
func (s ExecutionStatus) String() string {
@@ -55,6 +56,8 @@ func (s ExecutionStatus) String() string {
5556
return "InvalidForkchoice"
5657
case ExecutionStatusBusy:
5758
return "Busy"
59+
case ExecutionStatusReorgTooDeep:
60+
return "ReorgTooDeep"
5861
default:
5962
return fmt.Sprintf("ExecutionStatus(%d)", int32(s))
6063
}

‎node/gointerfaces/executionproto/execution.pb.go‎

Lines changed: 6 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎node/interfaces/execution/execution.proto‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ enum ExecutionStatus {
1313
TooFarAway = 2;
1414
MissingSegment = 3;
1515
InvalidForkchoice = 4;
16-
Busy = 5;
16+
Busy = 5;
17+
ReorgTooDeep = 6;
1718
}
1819

1920
message ForkChoiceReceipt {

0 commit comments

Comments
 (0)