Skip to content

Commit

Permalink
downloader: extract findAncestor search functions
Browse files Browse the repository at this point in the history
This is a simple refactoring, extracting common ancestor
negotiation logic to named functions.

This commit addresses the concern at the following link:
ethereum#21063 (comment)

---

This is a combination of 6 commits.
This is the 1st commit message:

downloader: extract span search to function

This is a plain refactoring, extracting span search
logic to its own function.

An error 'errNoAncestor' is introduced to handle
the case when the function does not find a common
ancestor.

Signed-off-by: meows <b5c6@protonmail.com>

downloader: extract binary search to function

This is a simple refactoring extracting
the binary search for common ancestor logic
to its own function.

A tweak was made to the 'floor' variable(s) which
I'm still not happy with, but logic is functional --
passing tests -- for now.

Signed-off-by: meows <b5c6@protonmail.com>

downloader: fixup floor type and logic to minimize change

This minimizes the diff, keeping the logic as nearly
the same as possible, limiting the span and binary search
logic extraction to functions as cleanly as possible.

Signed-off-by: meows <b5c6@protonmail.com>

downloader: remove limiting ancestor to localheight

This is logic not existing at ethereum/go-ethereum master,
and thus is not pertinent to a just-refactor.

Signed-off-by: meows <b5c6@protonmail.com>

downloader: tweaks to get the diff as clean as possible

Signed-off-by: meows <b5c6@protonmail.com>

downloader: rename error noAncestor -> noAncestorFound

Signed-off-by: meows <b5c6@protonmail.com>

sq
  • Loading branch information
meowsbits authored and holiman committed Jan 20, 2021
1 parent 034ecc3 commit bc4b7a1
Showing 1 changed file with 27 additions and 0 deletions.
27 changes: 27 additions & 0 deletions eth/downloader/downloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ var (
errCanceled = errors.New("syncing canceled (requested)")
errNoSyncActive = errors.New("no sync active")
errTooOld = errors.New("peer's protocol version too old")
errNoAncestorFound = errors.New("no common ancestor found")
)

type Downloader struct {
Expand Down Expand Up @@ -814,6 +815,26 @@ func (d *Downloader) findAncestor(p *peerConnection, remoteHeader *types.Header)
}
}

ancestor, err := d.findAncestorSpanSearch(p, mode, remoteHeight, localHeight, floor)
if err == nil {
return ancestor, nil
}
// The returned error was not nil.
// If the error returned does not reflect that a common ancestor was not found, return it.
// If the error reflects that a common ancestor was not found, continue to binary search,
// where the error value will be reassigned.
if err != errNoAncestorFound {
return 0, err
}

ancestor, err = d.findAncestorBinarySearch(p, mode, remoteHeight, floor)
if err != nil {
return 0, err
}
return ancestor, nil
}

func (d *Downloader) findAncestorSpanSearch(p *peerConnection, mode SyncMode, remoteHeight, localHeight uint64, floor int64) (commonAncestor uint64, err error) {
from, count, skip, max := calculateRequestSpan(remoteHeight, localHeight)

p.log.Trace("Span searching for common ancestor", "count", count, "from", from, "skip", skip)
Expand Down Expand Up @@ -894,6 +915,12 @@ func (d *Downloader) findAncestor(p *peerConnection, remoteHeader *types.Header)
p.log.Debug("Found common ancestor", "number", number, "hash", hash)
return number, nil
}
return 0, errNoAncestorFound
}

func (d *Downloader) findAncestorBinarySearch(p *peerConnection, mode SyncMode, remoteHeight uint64, floor int64) (commonAncestor uint64, err error) {
hash := common.Hash{}

// Ancestor not found, we need to binary search over our chain
start, end := uint64(0), remoteHeight
if floor > 0 {
Expand Down

0 comments on commit bc4b7a1

Please sign in to comment.