From 816ad30eff1a2468d184f21b10606944912c7628 Mon Sep 17 00:00:00 2001
From: Andrew Ashikhmin <34320705+yperbasis@users.noreply.github.com>
Date: Tue, 29 Mar 2022 11:47:07 +0200
Subject: [PATCH] Small Merge-related clarifications & improvements (#3785)
* Hd.Fetching -> FetchingNew
* ReadHeadHeaderHash makes more sense in verifyAndSaveNewPoSHeader
* Potentially more correct LatestValidHash in case of payload with invalid transactions
---
cmd/sentry/sentry/downloader.go | 2 +-
eth/stagedsync/stage_headers.go | 34 +++++++++----------
turbo/stages/headerdownload/header_algos.go | 8 ++---
.../headerdownload/header_data_struct.go | 2 +-
4 files changed, 22 insertions(+), 24 deletions(-)
diff --git a/cmd/sentry/sentry/downloader.go b/cmd/sentry/sentry/downloader.go
index 13b4041574..78d9f0f496 100644
--- a/cmd/sentry/sentry/downloader.go
+++ b/cmd/sentry/sentry/downloader.go
@@ -383,7 +383,7 @@ func NewControlServer(db kv.RwDB, nodeName string, chainConfig *params.ChainConf
}
func (cs *ControlServerImpl) newBlockHashes66(ctx context.Context, req *proto_sentry.InboundMessage, sentry direct.SentryClient) error {
- if !cs.Hd.RequestChaining() && !cs.Hd.Fetching() {
+ if !cs.Hd.RequestChaining() && !cs.Hd.FetchingNew() {
return nil
}
//log.Info(fmt.Sprintf("NewBlockHashes from [%s]", ConvertH256ToPeerID(req.PeerId)))
diff --git a/eth/stagedsync/stage_headers.go b/eth/stagedsync/stage_headers.go
index f12060b95d..4253330425 100644
--- a/eth/stagedsync/stage_headers.go
+++ b/eth/stagedsync/stage_headers.go
@@ -340,32 +340,30 @@ func handleNewPayload(
return nil
}
+ parent := rawdb.ReadHeader(tx, header.ParentHash, headerNumber-1)
+ if parent == nil {
+ log.Info(fmt.Sprintf("[%s] New payload missing parent", s.LogPrefix()))
+ hashToDownload := header.ParentHash
+ heightToDownload := headerNumber - 1
+ schedulePoSDownload(requestStatus, requestId, hashToDownload, heightToDownload, s, cfg)
+ return nil
+ }
+
+ cfg.hd.BeaconRequestList.Remove(requestId)
+
transactions, err := types.DecodeTransactions(payloadMessage.Body.Transactions)
if err != nil {
log.Warn("Error during Beacon transaction decoding", "err", err.Error())
- cfg.hd.BeaconRequestList.Remove(requestId)
if requestStatus == engineapi.New {
cfg.hd.PayloadStatusCh <- privateapi.PayloadStatus{
Status: remote.EngineStatus_INVALID,
- LatestValidHash: header.ParentHash, // TODO(yperbasis): potentially wrong when parent is nil
+ LatestValidHash: header.ParentHash,
ValidationError: err,
}
}
return nil
}
- parent := rawdb.ReadHeader(tx, header.ParentHash, headerNumber-1)
-
- if parent == nil {
- log.Info(fmt.Sprintf("[%s] New payload missing parent", s.LogPrefix()))
- hashToDownload := header.ParentHash
- heightToDownload := headerNumber - 1
- schedulePoSDownload(requestStatus, requestId, hashToDownload, heightToDownload, s, cfg)
- return nil
- }
-
- cfg.hd.BeaconRequestList.Remove(requestId)
-
log.Trace(fmt.Sprintf("[%s] New payload begin verification", s.LogPrefix()))
success, err := verifyAndSaveNewPoSHeader(requestStatus, s, tx, cfg, header, headerInserter)
log.Trace(fmt.Sprintf("[%s] New payload verification ended", s.LogPrefix()), "success", success, "err", err)
@@ -412,8 +410,8 @@ func verifyAndSaveNewPoSHeader(
return
}
- headBlockHash := rawdb.ReadHeadBlockHash(tx)
- if headBlockHash == header.ParentHash {
+ currentHeadHash := rawdb.ReadHeadHeaderHash(tx)
+ if currentHeadHash == header.ParentHash {
// OK, we're on the canonical chain
if requestStatus == engineapi.New {
cfg.hd.SetPendingPayloadStatus(headerHash)
@@ -539,8 +537,8 @@ func HeadersPOW(
return err
}
cfg.hd.SetPOSSync(false)
- cfg.hd.SetFetching(true)
- defer cfg.hd.SetFetching(false)
+ cfg.hd.SetFetchingNew(true)
+ defer cfg.hd.SetFetchingNew(false)
headerProgress = cfg.hd.Progress()
logPrefix := s.LogPrefix()
// Check if this is called straight after the unwinds, which means we need to create new canonical markings
diff --git a/turbo/stages/headerdownload/header_algos.go b/turbo/stages/headerdownload/header_algos.go
index 87e15b6432..ff7178dbbf 100644
--- a/turbo/stages/headerdownload/header_algos.go
+++ b/turbo/stages/headerdownload/header_algos.go
@@ -1217,10 +1217,10 @@ func (hd *HeaderDownload) EnableRequestChaining() {
hd.requestChaining = true
}
-func (hd *HeaderDownload) SetFetching(fetching bool) {
+func (hd *HeaderDownload) SetFetchingNew(fetching bool) {
hd.lock.Lock()
defer hd.lock.Unlock()
- hd.fetching = fetching
+ hd.fetchingNew = fetching
}
func (hd *HeaderDownload) SetPosStatus(status SyncStatus) {
@@ -1265,10 +1265,10 @@ func (hd *HeaderDownload) RequestChaining() bool {
return hd.requestChaining
}
-func (hd *HeaderDownload) Fetching() bool {
+func (hd *HeaderDownload) FetchingNew() bool {
hd.lock.RLock()
defer hd.lock.RUnlock()
- return hd.fetching
+ return hd.fetchingNew
}
func (hd *HeaderDownload) GetPendingPayloadStatus() common.Hash {
diff --git a/turbo/stages/headerdownload/header_data_struct.go b/turbo/stages/headerdownload/header_data_struct.go
index 4fed6e9d60..c3dd582378 100644
--- a/turbo/stages/headerdownload/header_data_struct.go
+++ b/turbo/stages/headerdownload/header_data_struct.go
@@ -277,7 +277,7 @@ type HeaderDownload struct {
anchorLimit int // Maximum allowed number of anchors
highestInDb uint64 // Height of the highest block header in the database
requestChaining bool // Whether the downloader is allowed to issue more requests when previous responses created or moved an anchor
- fetching bool // Set when the stage that is actively fetching the headers is in progress
+ fetchingNew bool // Set when the stage that is actively fetching the headers is in progress
topSeenHeightPoW uint64
consensusHeaderReader consensus.ChainHeaderReader
--
GitLab