Skip to content

eth/handler: more reliable announcements #22249

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

eth/handler: more reliable announcements #22249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 29 additions & 7 deletions eth/fetcher/block_fetcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,7 @@ func (f *BlockFetcher) loop() {
} else {
f.importBlocks(op.origin, op.block)
}
height = f.chainHeight()
}
// Wait for an outside event to occur
select {
Expand Down Expand Up @@ -798,15 +799,35 @@ func (f *BlockFetcher) importBlocks(peer string, block *types.Block) {
return
}
// Quickly validate the header and propagate the block if it passes
var announce bool
switch err := f.verifyHeader(block.Header()); err {
case nil:
// All ok, quickly propagate to our peers
blockBroadcastOutTimer.UpdateSince(block.ReceivedAt)
go f.broadcastBlock(block, true)

announce = true
case consensus.ErrFutureBlock:
// Weird future block, don't fail, but neither propagate

// Future block, don't fail, but neither propagate.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Future block, don't fail, but neither propagate.
// Future block, don't fail, but also don't propagate.

slightly clearer?

//
// Import it to the chain (it will internally be scheduled for later),
// but by default, don't announce it.
// If we do announce it, it will be marked as 'known' by the peer, but since it's
// not actually present in our chain, it the actual announcement will be
// skipped (see BroadcastBlock in eth/handler.go)
//
// This has a fairly high risk of happening on Clique-networks, where the
// Clique engine is a lot stricter about 'future' blocks -- ethash allows
// up to 15s "drift".
//
// To handle the Clique case better, we can handle some slack here:
// wait up to 2 seconds before trying to import + announce the block.
announce = false
if diff := int64(block.Header().Time) - time.Now().Unix(); diff > 0 && diff <= 2 {
// Less than two second, let's wait and then try to import
log.Info("Waiting before import + announce", "seconds", diff)
time.Sleep(time.Duration(diff) * time.Second)
announce = true
}
default:
// Something went very wrong, drop the peer
log.Debug("Propagated block verification failed", "peer", peer, "number", block.Number(), "hash", hash, "err", err)
Expand All @@ -818,10 +839,11 @@ func (f *BlockFetcher) importBlocks(peer string, block *types.Block) {
log.Debug("Propagated block import failed", "peer", peer, "number", block.Number(), "hash", hash, "err", err)
return
}
// If import succeeded, broadcast the block
blockAnnounceOutTimer.UpdateSince(block.ReceivedAt)
go f.broadcastBlock(block, false)

if announce {
// If import succeeded, broadcast the block
blockAnnounceOutTimer.UpdateSince(block.ReceivedAt)
go f.broadcastBlock(block, false)
}
// Invoke the testing hook if needed
if f.importedHook != nil {
f.importedHook(nil, block)
Expand Down
10 changes: 8 additions & 2 deletions eth/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,15 +450,21 @@ func (h *handler) BroadcastBlock(block *types.Block, propagate bool) {
for _, peer := range transfer {
peer.AsyncSendNewBlock(block, td)
}
log.Trace("Propagated block", "hash", hash, "recipients", len(transfer), "duration", common.PrettyDuration(time.Since(block.ReceivedAt)))
if len(transfer) > 0 {
log.Debug("Propagated block", "number", block.NumberU64(), "hash", hash, "recipients", len(transfer), "duration", common.PrettyDuration(time.Since(block.ReceivedAt)))
}
return
}
// Otherwise if the block is indeed in out own chain, announce it
if h.chain.HasBlock(hash, block.NumberU64()) {
for _, peer := range peers {
peer.AsyncSendNewBlockHash(block)
}
log.Trace("Announced block", "hash", hash, "recipients", len(peers), "duration", common.PrettyDuration(time.Since(block.ReceivedAt)))
if len(peers) > 0 {
log.Debug("Announced block", "number", block.NumberU64(), "hash", hash, "recipients", len(peers), "duration", common.PrettyDuration(time.Since(block.ReceivedAt)))
}
}else{
log.Debug("Block neither announced nor propagated","number", block.NumberU64(), "hash", hash)
}
}

Expand Down