From 9064038a86eead5f181d065a8ceb9ca2dc09fdbc Mon Sep 17 00:00:00 2001 From: Felix Lange Date: Tue, 11 Feb 2025 13:44:25 +0100 Subject: [PATCH] consensus/beacon: remove TestingTTDBlock (#31153) This removes the method `TestingTTDBlock` introduced by #30744. It was added to make the beacon consensus engine aware of the merge block in tests without relying on the total difficulty. However, tracking the merge block this way is very annoying. We usually configure forks in the `ChainConfig`, but the method is on the consensus engine, which isn't always created in the same place. By sidestepping the `ChainConfig` we don't get the usual fork-order checking, so it's possible to enable the merge before the London fork, for example. This in turn can lead to very hard-to-debug outputs and validation errors. So here I'm changing the consensus engine to check the `MergeNetsplitBlock` instead. Alternatively, we assume a network is merged if it has a `TerminalTotalDifficulty` of zero, which is a very common configuration in tests. --- consensus/beacon/consensus.go | 39 +++++-------------- eth/catalyst/api_test.go | 5 ++- eth/gasprice/gasprice_test.go | 11 +++--- eth/protocols/eth/handler_test.go | 4 +- eth/tracers/internal/tracetest/supply_test.go | 3 -- graphql/graphql_test.go | 7 +--- 6 files changed, 21 insertions(+), 48 deletions(-) diff --git a/consensus/beacon/consensus.go b/consensus/beacon/consensus.go index bdfddfff06..d80d24e272 100644 --- a/consensus/beacon/consensus.go +++ b/consensus/beacon/consensus.go @@ -61,8 +61,7 @@ var ( // is only used for necessary consensus checks. The legacy consensus engine can be any // engine implements the consensus interface (except the beacon itself). type Beacon struct { - ethone consensus.Engine // Original consensus engine used in eth1, e.g. ethash or clique - ttdblock *uint64 // Merge block-number for testchain generation without TTDs + ethone consensus.Engine // Original consensus engine used in eth1, e.g. ethash or clique } // New creates a consensus engine with the given embedded eth1 engine. @@ -73,16 +72,12 @@ func New(ethone consensus.Engine) *Beacon { return &Beacon{ethone: ethone} } -// TestingTTDBlock is a replacement mechanism for TTD-based pre-/post-merge -// splitting. With chain history deletion, TD calculations become impossible. -// This is fine for progressing the live chain, but to be able to generate test -// chains, we do need a split point. This method supports setting an explicit -// block number to use as the splitter *for testing*, instead of having to keep -// the notion of TDs in the client just for testing. -// -// The block with supplied number is regarded as the last pre-merge block. -func (beacon *Beacon) TestingTTDBlock(number uint64) { - beacon.ttdblock = &number +// isPostMerge reports whether the given block number is assumed to be post-merge. +// Here we check the MergeNetsplitBlock to allow configuring networks with a PoW or +// PoA chain for unit testing purposes. +func isPostMerge(config *params.ChainConfig, block uint64) bool { + mergedAtGenesis := config.TerminalTotalDifficulty != nil && config.TerminalTotalDifficulty.Sign() == 0 + return mergedAtGenesis || config.MergeNetsplitBlock != nil && block >= config.MergeNetsplitBlock.Uint64() } // Author implements consensus.Engine, returning the verified author of the block. @@ -332,15 +327,7 @@ func (beacon *Beacon) verifyHeaders(chain consensus.ChainHeaderReader, headers [ // Prepare implements consensus.Engine, initializing the difficulty field of a // header to conform to the beacon protocol. The changes are done inline. func (beacon *Beacon) Prepare(chain consensus.ChainHeaderReader, header *types.Header) error { - // The beacon engine requires access to total difficulties to be able to - // seal pre-merge and post-merge blocks. With the transition to removing - // old blocks, TDs become unaccessible, thus making TTD based pre-/post- - // merge decisions impossible. - // - // We do not need to seal non-merge blocks anymore live, but we do need - // to be able to generate test chains, thus we're reverting to a testing- - // settable field to direct that. - if beacon.ttdblock != nil && *beacon.ttdblock >= header.Number.Uint64() { + if !isPostMerge(chain.Config(), header.Number.Uint64()) { return beacon.ethone.Prepare(chain, header) } header.Difficulty = beaconDifficulty @@ -450,15 +437,7 @@ func (beacon *Beacon) SealHash(header *types.Header) common.Hash { // the difficulty that a new block should have when created at time // given the parent block's time and difficulty. func (beacon *Beacon) CalcDifficulty(chain consensus.ChainHeaderReader, time uint64, parent *types.Header) *big.Int { - // The beacon engine requires access to total difficulties to be able to - // seal pre-merge and post-merge blocks. With the transition to removing - // old blocks, TDs become unaccessible, thus making TTD based pre-/post- - // merge decisions impossible. - // - // We do not need to seal non-merge blocks anymore live, but we do need - // to be able to generate test chains, thus we're reverting to a testing- - // settable field to direct that. - if beacon.ttdblock != nil && *beacon.ttdblock > parent.Number.Uint64() { + if !isPostMerge(chain.Config(), parent.Number.Uint64()+1) { return beacon.ethone.CalcDifficulty(chain, time, parent) } return beaconDifficulty diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index 8fc3361192..e91e07d05d 100644 --- a/eth/catalyst/api_test.go +++ b/eth/catalyst/api_test.go @@ -64,9 +64,12 @@ func generateMergeChain(n int, merged bool) (*core.Genesis, []*types.Block) { engine := beacon.New(ethash.NewFaker()) if merged { config.TerminalTotalDifficulty = common.Big0 + config.MergeNetsplitBlock = common.Big0 } else { - engine.TestingTTDBlock(uint64(n)) + // When !merged, the tests expect the next block after the generated chain to be in PoS. + config.MergeNetsplitBlock = big.NewInt(int64(n + 1)) } + genesis := &core.Genesis{ Config: &config, Alloc: types.GenesisAlloc{ diff --git a/eth/gasprice/gasprice_test.go b/eth/gasprice/gasprice_test.go index 0a32c278cb..8e6524446f 100644 --- a/eth/gasprice/gasprice_test.go +++ b/eth/gasprice/gasprice_test.go @@ -147,11 +147,11 @@ func newTestBackend(t *testing.T, londonBlock *big.Int, cancunBlock *big.Int, pe config.LondonBlock = londonBlock config.ArrowGlacierBlock = londonBlock config.GrayGlacierBlock = londonBlock - + if cancunBlock != nil { + // Enable the merge with cancun fork. + config.MergeNetsplitBlock = cancunBlock + } engine := beacon.New(ethash.NewFaker()) - engine.TestingTTDBlock(testHead + 1) - - td := params.GenesisDifficulty.Uint64() if cancunBlock != nil { ts := gspec.Timestamp + cancunBlock.Uint64()*10 // fixed 10 sec block time in blockgen @@ -209,10 +209,9 @@ func newTestBackend(t *testing.T, londonBlock *big.Int, cancunBlock *big.Int, pe b.AddTx(types.MustSignNewTx(key, signer, blobTx)) } } - td += b.Difficulty().Uint64() }) + // Construct testing chain - gspec.Config.TerminalTotalDifficulty = new(big.Int).SetUint64(td) chain, err := core.NewBlockChain(db, &core.CacheConfig{TrieCleanNoPrefetch: true}, gspec, nil, engine, vm.Config{}, nil) if err != nil { t.Fatalf("Failed to create local chain, %v", err) diff --git a/eth/protocols/eth/handler_test.go b/eth/protocols/eth/handler_test.go index 5b35a2f935..f92599dba7 100644 --- a/eth/protocols/eth/handler_test.go +++ b/eth/protocols/eth/handler_test.go @@ -74,9 +74,7 @@ func newTestBackendWithGenerator(blocks int, shanghai bool, generator func(int, config = params.TestChainConfig engine = beacon.New(ethash.NewFaker()) ) - if !shanghai { - engine.TestingTTDBlock(math.MaxUint64) - } else { + if shanghai { config = ¶ms.ChainConfig{ ChainID: big.NewInt(1), HomesteadBlock: big.NewInt(0), diff --git a/eth/tracers/internal/tracetest/supply_test.go b/eth/tracers/internal/tracetest/supply_test.go index 3d54ab1868..57ba628b78 100644 --- a/eth/tracers/internal/tracetest/supply_test.go +++ b/eth/tracers/internal/tracetest/supply_test.go @@ -75,8 +75,6 @@ func TestSupplyOmittedFields(t *testing.T) { } ) - gspec.Config.TerminalTotalDifficulty = big.NewInt(0) - out, _, err := testSupplyTracer(t, gspec, func(b *core.BlockGen) { b.SetPoS() }) @@ -546,7 +544,6 @@ func TestSupplySelfdestructItselfAndRevert(t *testing.T) { func testSupplyTracer(t *testing.T, genesis *core.Genesis, gen func(*core.BlockGen)) ([]supplyInfo, *core.BlockChain, error) { engine := beacon.New(ethash.NewFaker()) - engine.TestingTTDBlock(1) traceOutputPath := filepath.ToSlash(t.TempDir()) traceOutputFilename := path.Join(traceOutputPath, "supply.jsonl") diff --git a/graphql/graphql_test.go b/graphql/graphql_test.go index 3e6a042dbb..0f6ba10b90 100644 --- a/graphql/graphql_test.go +++ b/graphql/graphql_test.go @@ -21,7 +21,6 @@ import ( "encoding/json" "fmt" "io" - "math" "math/big" "net/http" "strings" @@ -459,15 +458,13 @@ func newGQLService(t *testing.T, stack *node.Node, shanghai bool, gspec *core.Ge var engine = beacon.New(ethash.NewFaker()) if shanghai { gspec.Config.TerminalTotalDifficulty = common.Big0 + gspec.Config.MergeNetsplitBlock = common.Big0 // GenerateChain will increment timestamps by 10. // Shanghai upgrade at block 1. shanghaiTime := uint64(5) gspec.Config.ShanghaiTime = &shanghaiTime - } else { - // set an arbitrary large ttd as chains are required to be known to be merged - gspec.Config.TerminalTotalDifficulty = big.NewInt(math.MaxInt64) - engine.TestingTTDBlock(math.MaxUint64) } + ethBackend, err := eth.New(stack, ethConf) if err != nil { t.Fatalf("could not create eth backend: %v", err)