From be8f8409bc7f0f3124b7ebd1480337e4b1942113 Mon Sep 17 00:00:00 2001 From: Nick Johnson Date: Mon, 3 Jul 2017 15:17:12 +0100 Subject: [PATCH] eth/downloader, les, light: Changes in response to review --- eth/downloader/downloader.go | 39 +++++++++++++++++-------------- eth/downloader/downloader_test.go | 2 +- eth/downloader/peer.go | 32 +++++++++++++++++++++---- les/handler.go | 15 +----------- light/lightchain.go | 15 ------------ 5 files changed, 52 insertions(+), 51 deletions(-) diff --git a/eth/downloader/downloader.go b/eth/downloader/downloader.go index 0861e917a..cf8e1becf 100644 --- a/eth/downloader/downloader.go +++ b/eth/downloader/downloader.go @@ -115,7 +115,7 @@ type Downloader struct { syncStatsLock sync.RWMutex // Lock protecting the sync stats fields lightchain LightChain - chain BlockChain + blockchain BlockChain // Callbacks dropPeer peerDropFn // Drops a peer for misbehaving @@ -212,7 +212,7 @@ func New(mode SyncMode, stateDb ethdb.Database, mux *event.TypeMux, chain BlockC peers: newPeerSet(), rttEstimate: uint64(rttMaxEstimate), rttConfidence: uint64(1000000), - chain: chain, + blockchain: chain, lightchain: lightchain, dropPeer: dropPeer, headerCh: make(chan dataPack, 1), @@ -246,9 +246,9 @@ func (d *Downloader) Progress() ethereum.SyncProgress { current := uint64(0) switch d.mode { case FullSync: - current = d.chain.CurrentBlock().NumberU64() + current = d.blockchain.CurrentBlock().NumberU64() case FastSync: - current = d.chain.CurrentFastBlock().NumberU64() + current = d.blockchain.CurrentFastBlock().NumberU64() case LightSync: current = d.lightchain.CurrentHeader().Number.Uint64() } @@ -281,6 +281,11 @@ func (d *Downloader) RegisterPeer(id string, version int, peer Peer) error { return nil } +// RegisterLightPeer injects a light client peer +func (d *Downloader) RegisterLightPeer(id string, version int, peer LightPeer) error { + return d.RegisterPeer(id, version, &lightPeerWrapper{peer}) +} + // UnregisterPeer remove a peer from the known list, preventing any action from // the specified peer. An effort is also made to return any pending fetches into // the queue. @@ -597,9 +602,9 @@ func (d *Downloader) findAncestor(p *peerConnection, height uint64) (uint64, err p.log.Debug("Looking for common ancestor", "local", ceil, "remote", height) if d.mode == FullSync { - ceil = d.chain.CurrentBlock().NumberU64() + ceil = d.blockchain.CurrentBlock().NumberU64() } else if d.mode == FastSync { - ceil = d.chain.CurrentFastBlock().NumberU64() + ceil = d.blockchain.CurrentFastBlock().NumberU64() } if ceil >= MaxForkAncestry { floor = int64(ceil - MaxForkAncestry) @@ -659,7 +664,7 @@ func (d *Downloader) findAncestor(p *peerConnection, height uint64) (uint64, err continue } // Otherwise check if we already know the header or not - if (d.mode == FullSync && d.chain.HasBlockAndState(headers[i].Hash())) || (d.mode != FullSync && d.lightchain.HasHeader(headers[i].Hash())) { + if (d.mode == FullSync && d.blockchain.HasBlockAndState(headers[i].Hash())) || (d.mode != FullSync && d.lightchain.HasHeader(headers[i].Hash())) { number, hash = headers[i].Number.Uint64(), headers[i].Hash() // If every header is known, even future ones, the peer straight out lied about its head @@ -724,7 +729,7 @@ func (d *Downloader) findAncestor(p *peerConnection, height uint64) (uint64, err arrived = true // Modify the search interval based on the response - if (d.mode == FullSync && !d.chain.HasBlockAndState(headers[0].Hash())) || (d.mode != FullSync && !d.lightchain.HasHeader(headers[0].Hash())) { + if (d.mode == FullSync && !d.blockchain.HasBlockAndState(headers[0].Hash())) || (d.mode != FullSync && !d.lightchain.HasHeader(headers[0].Hash())) { end = check break } @@ -1147,14 +1152,14 @@ func (d *Downloader) processHeaders(origin uint64, td *big.Int) error { } lastHeader, lastFastBlock, lastBlock := d.lightchain.CurrentHeader().Number, common.Big0, common.Big0 if d.mode != LightSync { - lastFastBlock = d.chain.CurrentFastBlock().Number() - lastBlock = d.chain.CurrentBlock().Number() + lastFastBlock = d.blockchain.CurrentFastBlock().Number() + lastBlock = d.blockchain.CurrentBlock().Number() } d.lightchain.Rollback(hashes) curFastBlock, curBlock := common.Big0, common.Big0 if d.mode != LightSync { - curFastBlock = d.chain.CurrentFastBlock().Number() - curBlock = d.chain.CurrentBlock().Number() + curFastBlock = d.blockchain.CurrentFastBlock().Number() + curBlock = d.blockchain.CurrentBlock().Number() } log.Warn("Rolled back headers", "count", len(hashes), "header", fmt.Sprintf("%d->%d", lastHeader, d.lightchain.CurrentHeader().Number), @@ -1207,7 +1212,7 @@ func (d *Downloader) processHeaders(origin uint64, td *big.Int) error { // L: Request new headers up from 11 (R's TD was higher, it must have something) // R: Nothing to give if d.mode != LightSync { - if !gotHeaders && td.Cmp(d.chain.GetTdByHash(d.chain.CurrentBlock().Hash())) > 0 { + if !gotHeaders && td.Cmp(d.blockchain.GetTdByHash(d.blockchain.CurrentBlock().Hash())) > 0 { return errStallingPeer } } @@ -1345,7 +1350,7 @@ func (d *Downloader) importBlockResults(results []*fetchResult) error { for i, result := range results[:items] { blocks[i] = types.NewBlockWithHeader(result.Header).WithBody(result.Transactions, result.Uncles) } - if index, err := d.chain.InsertChain(blocks); err != nil { + if index, err := d.blockchain.InsertChain(blocks); err != nil { log.Debug("Downloaded item processing failed", "number", results[index].Header.Number, "hash", results[index].Header.Hash(), "err", err) return errInvalidChain } @@ -1434,7 +1439,7 @@ func (d *Downloader) commitFastSyncData(results []*fetchResult, stateSync *state blocks[i] = types.NewBlockWithHeader(result.Header).WithBody(result.Transactions, result.Uncles) receipts[i] = result.Receipts } - if index, err := d.chain.InsertReceiptChain(blocks, receipts); err != nil { + if index, err := d.blockchain.InsertReceiptChain(blocks, receipts); err != nil { log.Debug("Downloaded item processing failed", "number", results[index].Header.Number, "hash", results[index].Header.Hash(), "err", err) return errInvalidChain } @@ -1452,10 +1457,10 @@ func (d *Downloader) commitPivotBlock(result *fetchResult) error { return err } log.Debug("Committing fast sync pivot as new head", "number", b.Number(), "hash", b.Hash()) - if _, err := d.chain.InsertReceiptChain([]*types.Block{b}, []types.Receipts{result.Receipts}); err != nil { + if _, err := d.blockchain.InsertReceiptChain([]*types.Block{b}, []types.Receipts{result.Receipts}); err != nil { return err } - return d.chain.FastSyncCommitHead(b.Hash()) + return d.blockchain.FastSyncCommitHead(b.Hash()) } // DeliverHeaders injects a new batch of block headers received from a remote diff --git a/eth/downloader/downloader_test.go b/eth/downloader/downloader_test.go index bad2851d4..b354682a1 100644 --- a/eth/downloader/downloader_test.go +++ b/eth/downloader/downloader_test.go @@ -288,7 +288,7 @@ func (dl *downloadTester) CurrentFastBlock() *types.Block { return dl.genesis } -// FastSynccommitHead manually sets the head block to a given hash. +// FastSyncCommitHead manually sets the head block to a given hash. func (dl *downloadTester) FastSyncCommitHead(hash common.Hash) error { // For now only check that the state trie is correct if block := dl.GetBlockByHash(hash); block != nil { diff --git a/eth/downloader/peer.go b/eth/downloader/peer.go index 71a692688..96870da50 100644 --- a/eth/downloader/peer.go +++ b/eth/downloader/peer.go @@ -75,19 +75,43 @@ type peerConnection struct { lock sync.RWMutex } -type Peer interface { +type LightPeer interface { Head() (common.Hash, *big.Int) RequestHeadersByHash(common.Hash, int, int, bool) error RequestHeadersByNumber(uint64, int, int, bool) error +} + +type Peer interface { + LightPeer RequestBodies([]common.Hash) error RequestReceipts([]common.Hash) error RequestNodeData([]common.Hash) error } -// newPeer create a new downloader peer, with specific hash and block retrieval -// mechanisms. -func newPeerConnection(id string, version int, peer Peer, logger log.Logger) *peerConnection { +// lightPeerWrapper wraps a LightPeer struct, stubbing out the Peer-only methods. +type lightPeerWrapper struct { + peer LightPeer +} +func (w *lightPeerWrapper) Head() (common.Hash, *big.Int) { return w.peer.Head() } +func (w *lightPeerWrapper) RequestHeadersByHash(h common.Hash, amount int, skip int, reverse bool) error { + return w.peer.RequestHeadersByHash(h, amount, skip, reverse) +} +func (w *lightPeerWrapper) RequestHeadersByNumber(i uint64, amount int, skip int, reverse bool) error { + return w.peer.RequestHeadersByNumber(i, amount, skip, reverse) +} +func (w *lightPeerWrapper) RequestBodies([]common.Hash) error { + panic("RequestBodies not supported in light client mode sync") +} +func (w *lightPeerWrapper) RequestReceipts([]common.Hash) error { + panic("RequestReceipts not supported in light client mode sync") +} +func (w *lightPeerWrapper) RequestNodeData([]common.Hash) error { + panic("RequestNodeData not supported in light client mode sync") +} + +// newPeerConnection creates a new downloader peer +func newPeerConnection(id string, version int, peer Peer, logger log.Logger) *peerConnection { return &peerConnection{ id: id, lacking: make(map[common.Hash]struct{}), diff --git a/les/handler.go b/les/handler.go index 967b31521..39045ecbe 100644 --- a/les/handler.go +++ b/les/handler.go @@ -895,26 +895,13 @@ func (pc *peerConnection) RequestHeadersByNumber(origin uint64, amount int, skip return nil } -func (pc *peerConnection) RequestBodies(hashes []common.Hash) error { - panic("RequestBodies not supported in light client mode sync") - -} -func (pc *peerConnection) RequestReceipts(hashes []common.Hash) error { - panic("RequestReceipts not supported in light client mode sync") - -} - -func (pc *peerConnection) RequestNodeData(hashes []common.Hash) error { - panic("RequestNodeData not supported in light client mode sync") -} - func (d *downloaderPeerNotify) registerPeer(p *peer) { pm := (*ProtocolManager)(d) pc := &peerConnection{ manager: pm, peer: p, } - pm.downloader.RegisterPeer(p.id, ethVersion, pc) + pm.downloader.RegisterLightPeer(p.id, ethVersion, pc) } func (d *downloaderPeerNotify) unregisterPeer(p *peer) { diff --git a/light/lightchain.go b/light/lightchain.go index ec7ca3a0c..87436f4a5 100644 --- a/light/lightchain.go +++ b/light/lightchain.go @@ -389,21 +389,6 @@ func (self *LightChain) CurrentHeader() *types.Header { return self.hc.CurrentHeader() } -// CurrentBlock exists for interface compatibility and always returns nil -func (self *LightChain) CurrentBlock() *types.Block { - return nil -} - -// CurrentFastBlock exists for interface compatibility and always returns nil -func (self *LightChain) CurrentFastBlock() *types.Block { - return nil -} - -// FastSyncCommitHead exists for interface compatibility and does nothing -func (self *LightChain) FastSyncCommitHead(h common.Hash) error { - return nil -} - // GetTd retrieves a block's total difficulty in the canonical chain from the // database by hash and number, caching it if found. func (self *LightChain) GetTd(hash common.Hash, number uint64) *big.Int {