extract whitelist interface

eth/downloader/downloader.go

@@ -30,6 +30,7 @@ import (
 	"github.com/ethereum/go-ethereum/core/rawdb"
 	"github.com/ethereum/go-ethereum/core/state/snapshot"
 	"github.com/ethereum/go-ethereum/core/types"
+	"github.com/ethereum/go-ethereum/eth/downloader/whitelist"
 	"github.com/ethereum/go-ethereum/eth/protocols/snap"
 	"github.com/ethereum/go-ethereum/ethdb"
 	"github.com/ethereum/go-ethereum/event"
@@ -78,7 +79,6 @@ var (
 	errCanceled                = errors.New("syncing canceled (requested)")
 	errTooOld                  = errors.New("peer's protocol version too old")
 	errNoAncestorFound         = errors.New("no common ancestor found")
-	errCheckpointMismatch      = errors.New("checkpoint mismatch")
 	ErrMergeTransition         = errors.New("legacy sync reached the merge")
 )
 
@@ -144,9 +144,7 @@ type Downloader struct {
 	quitCh   chan struct{} // Quit channel to signal termination
 	quitLock sync.Mutex    // Lock to prevent double closes
 
-	// Checkpoint whitelist
+	ChainValidator
-	checkpointWhitelist map[uint64]common.Hash // Checkpoint whitelist, populated by reaching out to heimdall
-	checkpointOrder     []uint64               // Checkpoint order, populated by reaching out to heimdall
 
 	// Testing hooks
 	syncInitHook     func(uint64, uint64)  // Method to call upon initiating a new sync run
@@ -155,6 +153,11 @@ type Downloader struct {
 	chainInsertHook  func([]*fetchResult)  // Method to call upon inserting a chain of blocks (possibly in multiple invocations)
 }
 
+// interface for whitelist service
+type ChainValidator interface {
+	IsValidChain(remoteHeader *types.Header, fetchHeadersByNumber func(number uint64, amount int, skip int, reverse bool) ([]*types.Header, []common.Hash, error)) (bool, error)
+}
+
 // LightChain encapsulates functions required to synchronise a light chain.
 type LightChain interface {
 	// HasHeader verifies a header's presence in the local chain.
@@ -209,7 +212,7 @@ type BlockChain interface {
 }
 
 // New creates a new downloader to fetch hashes and blocks from remote peers.
-func New(checkpoint uint64, stateDb ethdb.Database, mux *event.TypeMux, chain BlockChain, lightchain LightChain, dropPeer peerDropFn, success func()) *Downloader {
+func New(checkpoint uint64, stateDb ethdb.Database, mux *event.TypeMux, chain BlockChain, lightchain LightChain, dropPeer peerDropFn, success func(), whitelistService ChainValidator) *Downloader {
 	if lightchain == nil {
 		lightchain = chain
 	}
@@ -226,8 +229,7 @@ func New(checkpoint uint64, stateDb ethdb.Database, mux *event.TypeMux, chain Bl
 		quitCh:              make(chan struct{}),
 		SnapSyncer:          snap.NewSyncer(stateDb),
 		stateSyncStart:      make(chan *stateSync),
-		checkpointWhitelist: make(map[uint64]common.Hash),
+		ChainValidator:		 whitelistService,
-		checkpointOrder:     make([]uint64, 0),
 	}
 	dl.skeleton = newSkeleton(stateDb, dl.peers, dropPeer, newBeaconBackfiller(dl, success))
 
@@ -339,7 +341,7 @@ func (d *Downloader) LegacySync(id string, head common.Hash, td, ttd *big.Int, m
 	case nil, errBusy, errCanceled:
 		return err
 	}
-	if errors.Is(err, errCheckpointMismatch) {
+	if errors.Is(err, whitelist.ErrCheckpointMismatch) {
 		// TODO: what better can be done here?
 		log.Warn("Mismatch in last checkpointed block", "peer", id, "err", err)
 	}
@@ -775,38 +777,11 @@ func calculateRequestSpan(remoteHeight, localHeight uint64) (int64, int, int, ui
 	return int64(from), count, span - 1, uint64(max)
 }
 
-// isValidChain checks if the chain we're about to receive from this peer is valid or not
+// curried fetchHeadersByNumber
-// in terms of reorgs. We won't reorg beyond the last bor checkpoint submitted to mainchain.
+func (d *Downloader) getFetchHeadersByNumber(p *peerConnection) func(number uint64, amount int, skip int, reverse bool) ([]*types.Header, []common.Hash, error) {
-func (d *Downloader) isValidChain(p *peerConnection, remoteHeader *types.Header) (bool, error) {
+	return func(number uint64, amount int, skip int, reverse bool) ([]*types.Header, []common.Hash, error) {
-	// We want to validate the chain by comparing the last checkpointed block
+		return d.fetchHeadersByNumber(p, number, amount, skip, reverse)
-	// we're storing in `checkpointWhitelist` with the peer's block.
-
-	// Check for availaibility of the last checkpointed block.
-	// This can be also be empty if our heimdall is not responsing
-	// or we're running without it.
-	if len(d.checkpointWhitelist) <= 0 {
-		// worst case, we don't have the checkpoints in memory
-		return true, nil
 	}
-
-	// Fetch the last checkpoint entry
-	lastCheckpointBlockNum := d.checkpointOrder[len(d.checkpointOrder)-1]
-	lastCheckpointBlockHash := d.checkpointWhitelist[lastCheckpointBlockNum]
-
-	headers, hashes, err := d.fetchHeadersByNumber(p, lastCheckpointBlockNum, 1, 0, false)
-	if err != nil || len(headers) == 0 {
-		// TODO: what better can be done here?
-		return true, nil
-	}
-	reqBlockNum := headers[0].Number.Uint64()
-	reqBlockHash := hashes[0]
-
-	// Check against the checkpointed blocks
-	if reqBlockNum == lastCheckpointBlockNum && reqBlockHash == lastCheckpointBlockHash {
-		return true, nil
-	}
-
-	return false, errCheckpointMismatch
 }
 
 // findAncestor tries to locate the common ancestor link of the local chain and
@@ -816,7 +791,8 @@ func (d *Downloader) isValidChain(p *peerConnection, remoteHeader *types.Header)
 // the head links match), we do a binary search to find the common ancestor.
 func (d *Downloader) findAncestor(p *peerConnection, remoteHeader *types.Header) (uint64, error) {
 	// Check the validity of chain to be downloaded
-	if _, err := d.isValidChain(p, remoteHeader); errors.Is(err, errCheckpointMismatch) {
+	// TODO: we can use a mock and 
+	if _, err := d.IsValidChain(remoteHeader, d.getFetchHeadersByNumber(p)); errors.Is(err, whitelist.ErrCheckpointMismatch) {
 		return 0, err
 	}
 
@@ -1805,36 +1781,3 @@ func (d *Downloader) DeliverSnapPacket(peer *snap.Peer, packet snap.Packet) erro
 		return fmt.Errorf("unexpected snap packet type: %T", packet)
 	}
 }
-
-// Helper functions for checkpoint whitelist service
-
-// PurgeWhitelistMap purges data from checkpoint whitelist map
-func (d *Downloader) PurgeWhitelistMap() error {
-	for k := range d.checkpointWhitelist {
-		delete(d.checkpointWhitelist, k)
-	}
-	return nil
-}
-
-// EnqueueWhitelistBlock enqueues blockNumber, blockHash to the checkpoint whitelist map
-func (d *Downloader) EnqueueCheckpointWhitelist(key uint64, val common.Hash) {
-	if _, ok := d.checkpointWhitelist[key]; !ok {
-		log.Debug("Enqueing new checkpoint whitelist", "block number", key, "block hash", val)
-		d.checkpointWhitelist[key] = val
-		d.checkpointOrder = append(d.checkpointOrder, key)
-	}
-}
-
-// DequeueWhitelistBlock dequeues block, blockhash from the checkpoint whitelist map
-func (d *Downloader) DequeueCheckpointWhitelist() {
-	if len(d.checkpointOrder) > 0 {
-		log.Debug("Dequeing checkpoint whitelist", "block number", d.checkpointOrder[0], "block hash", d.checkpointWhitelist[d.checkpointOrder[0]])
-		delete(d.checkpointWhitelist, d.checkpointOrder[0])
-		d.checkpointOrder = d.checkpointOrder[1:]
-	}
-}
-
-// GetCheckpointWhitelist returns the checkpoints whitelisted.
-func (d *Downloader) GetCheckpointWhitelist() map[uint64]common.Hash {
-	return d.checkpointWhitelist
-}

eth/downloader/downloader_test.go

@@ -35,6 +35,7 @@ import (
 	"github.com/ethereum/go-ethereum/core/rawdb"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/core/vm"
+	"github.com/ethereum/go-ethereum/eth/downloader/whitelist"
 	"github.com/ethereum/go-ethereum/eth/protocols/eth"
 	"github.com/ethereum/go-ethereum/eth/protocols/snap"
 	"github.com/ethereum/go-ethereum/event"
@@ -75,7 +76,9 @@ func newTester() *downloadTester {
 		chain:   chain,
 		peers:   make(map[string]*downloadTesterPeer),
 	}
-	tester.downloader = New(0, db, new(event.TypeMux), tester.chain, nil, tester.dropPeer, nil)
+
+	// TODO: here we can inject a mock
+	tester.downloader = New(0, db, new(event.TypeMux), tester.chain, nil, tester.dropPeer, nil, whitelist.NewService())
 	return tester
 }
 

eth/downloader/whitelist/service.go

@@ -0,0 +1,93 @@
+package whitelist
+
+import (
+	"errors"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/core/types"
+	"github.com/ethereum/go-ethereum/log"
+)
+
+var (
+	ErrCheckpointMismatch = errors.New("checkpoint mismatch")
+)
+
+type Service struct {
+	// Checkpoint whitelist
+	checkpointWhitelist map[uint64]common.Hash // Checkpoint whitelist, populated by reaching out to heimdall
+	checkpointOrder     []uint64               // Checkpoint order, populated by reaching out to heimdall
+}
+
+func NewService() *Service {
+	return &Service{
+		checkpointWhitelist: make(map[uint64]common.Hash),
+		checkpointOrder:     []uint64{},
+	}
+}
+
+// IsValidChain checks if the chain we're about to receive from this peer is valid or not
+// in terms of reorgs. We won't reorg beyond the last bor checkpoint submitted to mainchain.
+func (w *Service) IsValidChain(remoteHeader *types.Header, fetchHeadersByNumber func(number uint64, amount int, skip int, reverse bool) ([]*types.Header, []common.Hash, error)) (bool, error) {
+	// We want to validate the chain by comparing the last checkpointed block
+	// we're storing in `checkpointWhitelist` with the peer's block.
+
+	// Check for availaibility of the last checkpointed block.
+	// This can be also be empty if our heimdall is not responsing
+	// or we're running without it.
+	if len(w.checkpointWhitelist) <= 0 {
+		// worst case, we don't have the checkpoints in memory
+		return true, nil
+	}
+
+	// Fetch the last checkpoint entry
+	lastCheckpointBlockNum := w.checkpointOrder[len(w.checkpointOrder)-1]
+	lastCheckpointBlockHash := w.checkpointWhitelist[lastCheckpointBlockNum]
+
+	// todo: we can extract this as an interface and mock as well or just test IsValidChain in isolation from downloader passing fake fetchHeadersByNumber functions
+	headers, hashes, err := fetchHeadersByNumber(lastCheckpointBlockNum, 1, 0, false)
+	if err != nil || len(headers) == 0 {
+		// TODO: what better can be done here?
+		return true, nil
+	}
+
+	reqBlockNum := headers[0].Number.Uint64()
+	reqBlockHash := hashes[0]
+
+	// Check against the checkpointed blocks
+	if reqBlockNum == lastCheckpointBlockNum && reqBlockHash == lastCheckpointBlockHash {
+		return true, nil
+	}
+
+	return false, ErrCheckpointMismatch
+}
+
+// PurgeWhitelistMap purges data from checkpoint whitelist map
+func (w *Service) PurgeWhitelistMap() error {
+	for k := range w.checkpointWhitelist {
+		delete(w.checkpointWhitelist, k)
+	}
+	return nil
+}
+
+// EnqueueWhitelistBlock enqueues blockNumber, blockHash to the checkpoint whitelist map
+func (w *Service) EnqueueCheckpointWhitelist(key uint64, val common.Hash) {
+	if _, ok := w.checkpointWhitelist[key]; !ok {
+		log.Debug("Enqueing new checkpoint whitelist", "block number", key, "block hash", val)
+		w.checkpointWhitelist[key] = val
+		w.checkpointOrder = append(w.checkpointOrder, key)
+	}
+}
+
+// DequeueWhitelistBlock dequeues block, blockhash from the checkpoint whitelist map
+func (w *Service) DequeueCheckpointWhitelist() {
+	if len(w.checkpointOrder) > 0 {
+		log.Debug("Dequeing checkpoint whitelist", "block number", w.checkpointOrder[0], "block hash", w.checkpointWhitelist[w.checkpointOrder[0]])
+		delete(w.checkpointWhitelist, w.checkpointOrder[0])
+		w.checkpointOrder = w.checkpointOrder[1:]
+	}
+}
+
+// GetCheckpointWhitelist returns the checkpoints whitelisted.
+func (w *Service) GetCheckpointWhitelist() map[uint64]common.Hash {
+	return w.checkpointWhitelist
+}