From 93fe17559b3f47bbd89fe0edb65e55a1cd130588 Mon Sep 17 00:00:00 2001 From: Marius van der Wijden Date: Mon, 30 May 2022 13:28:15 +0200 Subject: [PATCH] eth/catalyst: fix edge case in NewPayload (#24955) Fixes an issue where we would accept a NewPayload where the grandparent is already post ttd, and the parent still has a Difficulty --- eth/catalyst/api.go | 19 ++++++++++---- eth/catalyst/api_test.go | 56 ++++++++++++++++++++++++++++++++++------ 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index 108ec412d9..5702726ee6 100644 --- a/eth/catalyst/api.go +++ b/eth/catalyst/api.go @@ -138,10 +138,14 @@ func (api *ConsensusAPI) ForkchoiceUpdatedV1(update beacon.ForkchoiceStateV1, pa log.Error("TDs unavailable for TTD check", "number", block.NumberU64(), "hash", update.HeadBlockHash, "td", td, "parent", block.ParentHash(), "ptd", ptd) return beacon.STATUS_INVALID, errors.New("TDs unavailable for TDD check") } - if td.Cmp(ttd) < 0 || (block.NumberU64() > 0 && ptd.Cmp(ttd) > 0) { + if td.Cmp(ttd) < 0 { log.Error("Refusing beacon update to pre-merge", "number", block.NumberU64(), "hash", update.HeadBlockHash, "diff", block.Difficulty(), "age", common.PrettyAge(time.Unix(int64(block.Time()), 0))) return beacon.ForkChoiceResponse{PayloadStatus: beacon.INVALID_TERMINAL_BLOCK, PayloadID: nil}, nil } + if block.NumberU64() > 0 && ptd.Cmp(ttd) >= 0 { + log.Error("Parent block is already post-ttd", "number", block.NumberU64(), "hash", update.HeadBlockHash, "diff", block.Difficulty(), "age", common.PrettyAge(time.Unix(int64(block.Time()), 0))) + return beacon.ForkChoiceResponse{PayloadStatus: beacon.INVALID_TERMINAL_BLOCK, PayloadID: nil}, nil + } } if rawdb.ReadCanonicalHash(api.eth.ChainDb(), block.NumberU64()) != update.HeadBlockHash { @@ -295,11 +299,16 @@ func (api *ConsensusAPI) NewPayloadV1(params beacon.ExecutableDataV1) (beacon.Pa // We have an existing parent, do some sanity checks to avoid the beacon client // triggering too early var ( - td = api.eth.BlockChain().GetTd(parent.Hash(), parent.NumberU64()) - ttd = api.eth.BlockChain().Config().TerminalTotalDifficulty + ptd = api.eth.BlockChain().GetTd(parent.Hash(), parent.NumberU64()) + ttd = api.eth.BlockChain().Config().TerminalTotalDifficulty + gptd = api.eth.BlockChain().GetTd(parent.ParentHash(), parent.NumberU64()-1) ) - if td.Cmp(ttd) < 0 { - log.Warn("Ignoring pre-merge payload", "number", params.Number, "hash", params.BlockHash, "td", td, "ttd", ttd) + if ptd.Cmp(ttd) < 0 { + log.Warn("Ignoring pre-merge payload", "number", params.Number, "hash", params.BlockHash, "td", ptd, "ttd", ttd) + return beacon.INVALID_TERMINAL_BLOCK, nil + } + if parent.Difficulty().BitLen() > 0 && gptd != nil && gptd.Cmp(ttd) >= 0 { + log.Error("Ignoring pre-merge parent block", "number", params.Number, "hash", params.BlockHash, "td", ptd, "ttd", ttd) return beacon.INVALID_TERMINAL_BLOCK, nil } if block.Time() <= parent.Time() { diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index 415506d58e..c80fb20344 100644 --- a/eth/catalyst/api_test.go +++ b/eth/catalyst/api_test.go @@ -205,7 +205,6 @@ func checkLogEvents(t *testing.T, logsCh <-chan []*types.Log, rmLogsCh <-chan co func TestInvalidPayloadTimestamp(t *testing.T) { genesis, preMergeBlocks := generatePreMergeChain(10) n, ethservice := startEthService(t, genesis, preMergeBlocks) - ethservice.Merger().ReachTTD() defer n.Close() var ( api = NewConsensusAPI(ethservice) @@ -250,7 +249,6 @@ func TestInvalidPayloadTimestamp(t *testing.T) { func TestEth2NewBlock(t *testing.T) { genesis, preMergeBlocks := generatePreMergeChain(10) n, ethservice := startEthService(t, genesis, preMergeBlocks) - ethservice.Merger().ReachTTD() defer n.Close() var ( @@ -427,7 +425,6 @@ func startEthService(t *testing.T, genesis *core.Genesis, blocks []*types.Block) func TestFullAPI(t *testing.T) { genesis, preMergeBlocks := generatePreMergeChain(10) n, ethservice := startEthService(t, genesis, preMergeBlocks) - ethservice.Merger().ReachTTD() defer n.Close() var ( parent = ethservice.BlockChain().CurrentBlock() @@ -480,7 +477,6 @@ func setupBlocks(t *testing.T, ethservice *eth.Ethereum, n int, parent *types.Bl func TestExchangeTransitionConfig(t *testing.T) { genesis, preMergeBlocks := generatePreMergeChain(10) n, ethservice := startEthService(t, genesis, preMergeBlocks) - ethservice.Merger().ReachTTD() defer n.Close() var ( api = NewConsensusAPI(ethservice) @@ -543,7 +539,6 @@ CommonAncestor◄─▲── P1 ◄── P2 ◄─ P3 ◄─ ... ◄─ Pn func TestNewPayloadOnInvalidChain(t *testing.T) { genesis, preMergeBlocks := generatePreMergeChain(10) n, ethservice := startEthService(t, genesis, preMergeBlocks) - ethservice.Merger().ReachTTD() defer n.Close() var ( @@ -618,7 +613,6 @@ func assembleBlock(api *ConsensusAPI, parentHash common.Hash, params *beacon.Pay func TestEmptyBlocks(t *testing.T) { genesis, preMergeBlocks := generatePreMergeChain(10) n, ethservice := startEthService(t, genesis, preMergeBlocks) - ethservice.Merger().ReachTTD() defer n.Close() commonAncestor := ethservice.BlockChain().CurrentBlock() @@ -734,8 +728,6 @@ func TestTrickRemoteBlockCache(t *testing.T) { genesis, preMergeBlocks := generatePreMergeChain(10) nodeA, ethserviceA := startEthService(t, genesis, preMergeBlocks) nodeB, ethserviceB := startEthService(t, genesis, preMergeBlocks) - ethserviceA.Merger().ReachTTD() - ethserviceB.Merger().ReachTTD() defer nodeA.Close() defer nodeB.Close() for nodeB.Server().NodeInfo().Ports.Listener == 0 { @@ -794,3 +786,51 @@ func TestTrickRemoteBlockCache(t *testing.T) { time.Sleep(100 * time.Millisecond) } } + +func TestNewPayloadOnInvalidTerminalBlock(t *testing.T) { + genesis, preMergeBlocks := generatePreMergeChain(100) + fmt.Println(genesis.Config.TerminalTotalDifficulty) + genesis.Config.TerminalTotalDifficulty = preMergeBlocks[0].Difficulty() //.Sub(genesis.Config.TerminalTotalDifficulty, preMergeBlocks[len(preMergeBlocks)-1].Difficulty()) + + fmt.Println(genesis.Config.TerminalTotalDifficulty) + n, ethservice := startEthService(t, genesis, preMergeBlocks) + defer n.Close() + + var ( + api = NewConsensusAPI(ethservice) + parent = preMergeBlocks[len(preMergeBlocks)-1] + ) + + // Test parent already post TTD in FCU + fcState := beacon.ForkchoiceStateV1{ + HeadBlockHash: parent.Hash(), + SafeBlockHash: common.Hash{}, + FinalizedBlockHash: common.Hash{}, + } + resp, err := api.ForkchoiceUpdatedV1(fcState, nil) + if err != nil { + t.Fatalf("error sending forkchoice, err=%v", err) + } + if resp.PayloadStatus != beacon.INVALID_TERMINAL_BLOCK { + t.Fatalf("error sending invalid forkchoice, invalid status: %v", resp.PayloadStatus.Status) + } + + // Test parent already post TTD in NewPayload + params := beacon.PayloadAttributesV1{ + Timestamp: parent.Time() + 1, + Random: crypto.Keccak256Hash([]byte{byte(1)}), + SuggestedFeeRecipient: parent.Coinbase(), + } + empty, err := api.eth.Miner().GetSealingBlockSync(parent.Hash(), params.Timestamp, params.SuggestedFeeRecipient, params.Random, true) + if err != nil { + t.Fatalf("error preparing payload, err=%v", err) + } + data := *beacon.BlockToExecutableData(empty) + resp2, err := api.NewPayloadV1(data) + if err != nil { + t.Fatalf("error sending NewPayload, err=%v", err) + } + if resp2 != beacon.INVALID_TERMINAL_BLOCK { + t.Fatalf("error sending invalid forkchoice, invalid status: %v", resp.PayloadStatus.Status) + } +}