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
pull/24997/head
Marius van der Wijden 3 years ago committed by GitHub
parent 8845227306
commit 93fe17559b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 19
      eth/catalyst/api.go
  2. 56
      eth/catalyst/api_test.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) 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") 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))) 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 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 { 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 // We have an existing parent, do some sanity checks to avoid the beacon client
// triggering too early // triggering too early
var ( var (
td = api.eth.BlockChain().GetTd(parent.Hash(), parent.NumberU64()) ptd = api.eth.BlockChain().GetTd(parent.Hash(), parent.NumberU64())
ttd = api.eth.BlockChain().Config().TerminalTotalDifficulty ttd = api.eth.BlockChain().Config().TerminalTotalDifficulty
gptd = api.eth.BlockChain().GetTd(parent.ParentHash(), parent.NumberU64()-1)
) )
if td.Cmp(ttd) < 0 { if ptd.Cmp(ttd) < 0 {
log.Warn("Ignoring pre-merge payload", "number", params.Number, "hash", params.BlockHash, "td", td, "ttd", ttd) 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 return beacon.INVALID_TERMINAL_BLOCK, nil
} }
if block.Time() <= parent.Time() { if block.Time() <= parent.Time() {

@ -205,7 +205,6 @@ func checkLogEvents(t *testing.T, logsCh <-chan []*types.Log, rmLogsCh <-chan co
func TestInvalidPayloadTimestamp(t *testing.T) { func TestInvalidPayloadTimestamp(t *testing.T) {
genesis, preMergeBlocks := generatePreMergeChain(10) genesis, preMergeBlocks := generatePreMergeChain(10)
n, ethservice := startEthService(t, genesis, preMergeBlocks) n, ethservice := startEthService(t, genesis, preMergeBlocks)
ethservice.Merger().ReachTTD()
defer n.Close() defer n.Close()
var ( var (
api = NewConsensusAPI(ethservice) api = NewConsensusAPI(ethservice)
@ -250,7 +249,6 @@ func TestInvalidPayloadTimestamp(t *testing.T) {
func TestEth2NewBlock(t *testing.T) { func TestEth2NewBlock(t *testing.T) {
genesis, preMergeBlocks := generatePreMergeChain(10) genesis, preMergeBlocks := generatePreMergeChain(10)
n, ethservice := startEthService(t, genesis, preMergeBlocks) n, ethservice := startEthService(t, genesis, preMergeBlocks)
ethservice.Merger().ReachTTD()
defer n.Close() defer n.Close()
var ( var (
@ -427,7 +425,6 @@ func startEthService(t *testing.T, genesis *core.Genesis, blocks []*types.Block)
func TestFullAPI(t *testing.T) { func TestFullAPI(t *testing.T) {
genesis, preMergeBlocks := generatePreMergeChain(10) genesis, preMergeBlocks := generatePreMergeChain(10)
n, ethservice := startEthService(t, genesis, preMergeBlocks) n, ethservice := startEthService(t, genesis, preMergeBlocks)
ethservice.Merger().ReachTTD()
defer n.Close() defer n.Close()
var ( var (
parent = ethservice.BlockChain().CurrentBlock() 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) { func TestExchangeTransitionConfig(t *testing.T) {
genesis, preMergeBlocks := generatePreMergeChain(10) genesis, preMergeBlocks := generatePreMergeChain(10)
n, ethservice := startEthService(t, genesis, preMergeBlocks) n, ethservice := startEthService(t, genesis, preMergeBlocks)
ethservice.Merger().ReachTTD()
defer n.Close() defer n.Close()
var ( var (
api = NewConsensusAPI(ethservice) api = NewConsensusAPI(ethservice)
@ -543,7 +539,6 @@ CommonAncestor◄─▲── P1 ◄── P2 ◄─ P3 ◄─ ... ◄─ Pn
func TestNewPayloadOnInvalidChain(t *testing.T) { func TestNewPayloadOnInvalidChain(t *testing.T) {
genesis, preMergeBlocks := generatePreMergeChain(10) genesis, preMergeBlocks := generatePreMergeChain(10)
n, ethservice := startEthService(t, genesis, preMergeBlocks) n, ethservice := startEthService(t, genesis, preMergeBlocks)
ethservice.Merger().ReachTTD()
defer n.Close() defer n.Close()
var ( var (
@ -618,7 +613,6 @@ func assembleBlock(api *ConsensusAPI, parentHash common.Hash, params *beacon.Pay
func TestEmptyBlocks(t *testing.T) { func TestEmptyBlocks(t *testing.T) {
genesis, preMergeBlocks := generatePreMergeChain(10) genesis, preMergeBlocks := generatePreMergeChain(10)
n, ethservice := startEthService(t, genesis, preMergeBlocks) n, ethservice := startEthService(t, genesis, preMergeBlocks)
ethservice.Merger().ReachTTD()
defer n.Close() defer n.Close()
commonAncestor := ethservice.BlockChain().CurrentBlock() commonAncestor := ethservice.BlockChain().CurrentBlock()
@ -734,8 +728,6 @@ func TestTrickRemoteBlockCache(t *testing.T) {
genesis, preMergeBlocks := generatePreMergeChain(10) genesis, preMergeBlocks := generatePreMergeChain(10)
nodeA, ethserviceA := startEthService(t, genesis, preMergeBlocks) nodeA, ethserviceA := startEthService(t, genesis, preMergeBlocks)
nodeB, ethserviceB := startEthService(t, genesis, preMergeBlocks) nodeB, ethserviceB := startEthService(t, genesis, preMergeBlocks)
ethserviceA.Merger().ReachTTD()
ethserviceB.Merger().ReachTTD()
defer nodeA.Close() defer nodeA.Close()
defer nodeB.Close() defer nodeB.Close()
for nodeB.Server().NodeInfo().Ports.Listener == 0 { for nodeB.Server().NodeInfo().Ports.Listener == 0 {
@ -794,3 +786,51 @@ func TestTrickRemoteBlockCache(t *testing.T) {
time.Sleep(100 * time.Millisecond) 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)
}
}

Loading…
Cancel
Save