eth/catalyst: fix edge case in NewPayload (ethereum#24955)

Fixes an issue where we would accept a NewPayload where the grandparent is already post ttd, and the parent still has a Difficulty

Author: MariusVanDerWijden

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() {

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)
+	}
+}