Fixed review comments

bzzeth/bzzeth.go

@@ -23,29 +23,23 @@ import (
 	"github.com/ethereum/go-ethereum/p2p"
 	"github.com/ethereum/go-ethereum/rpc"
 	"github.com/ethersphere/swarm/log"
-	"github.com/ethersphere/swarm/network"
 	"github.com/ethersphere/swarm/p2p/protocols"
-	"github.com/ethersphere/swarm/storage"
 )
 
 // BzzEth implements node.Service
 var _ node.Service = &BzzEth{}
 
 // BzzEth is a global module handling ethereum state on swarm
 type BzzEth struct {
-	peers    *peers            // bzzeth peer pool
-	netStore *storage.NetStore // netstore to retrieve and store
-	kad      *network.Kademlia // kademlia to determine if a header chunk belongs to us
-	quit     chan struct{}     // quit channel to close go routines
+	peers *peers        // bzzeth peer pool
+	quit  chan struct{} // quit channel to close go routines
 }
 
 // New constructs the BzzEth node service
-func New(ns *storage.NetStore, kad *network.Kademlia) *BzzEth {
+func New() *BzzEth {
 	return &BzzEth{
-		peers:    newPeers(),
-		netStore: ns,
-		kad:      kad,
-		quit:     make(chan struct{}),
+		peers: newPeers(),
+		quit:  make(chan struct{}),
 	}
 }
 
@@ -65,8 +59,10 @@ func (b *BzzEth) Run(p *p2p.Peer, rw p2p.MsgReadWriter) error {
 		return err
 	}
 	bp.serveHeaders = handshake.(*Handshake).ServeHeaders
-	log.Warn("handshake", "hs", handshake, "peer", bp)
-	// with another swarm node the protocol goes into idle
+	log.Debug("handshake", "hs", handshake, "peer", bp)
+
+	// This protocol is all about interaction between an Eth node and a Swarm Node.
+	// If another swarm node tries to connect then the protocol goes into idle
 	if isSwarmNodeFunc(bp) {
 		<-b.quit
 		return nil
@@ -84,7 +80,6 @@ func (b *BzzEth) handleMsg(p *Peer) func(context.Context, interface{}) error {
 		p.logger.Debug("bzzeth.handleMsg")
 		switch msg.(type) {
 		default:
-			log.Info("Received a message ")
 		}
 		return nil
 	}

bzzeth/bzzeth_test.go

@@ -41,7 +41,7 @@ func init() {
 }
 
 func newBzzEthTester() (*p2ptest.ProtocolTester, *BzzEth, func(), error) {
-	b := New(nil, nil)
+	b := New()
 
 	prvkey, err := crypto.GenerateKey()
 	if err != nil {

bzzeth/peer.go

@@ -60,27 +60,26 @@ func (p *peers) get(id enode.ID) *Peer {
 
 func (p *peers) add(peer *Peer) {
 	p.mtx.Lock()
-	defer p.mtx.Unlock()
 	p.peers[peer.ID()] = peer
+	p.mtx.Unlock()
 }
 
 func (p *peers) remove(peer *Peer) {
 	p.mtx.Lock()
-	defer p.mtx.Unlock()
 	delete(p.peers, peer.ID())
+	p.mtx.Unlock()
 }
 
 // getEthPeer finds a peer that serves headers and calls the function argument on this peer
-// TODO: implement load balancing of requests in case of multiple peers
-func (p *peers) getEth() (peer *Peer) {
+func (p *peers) getEth() *Peer {
 	p.mtx.RLock()
 	defer p.mtx.RUnlock()
-	for _, peer = range p.peers {
+	for _, peer := range p.peers {
 		if peer.serveHeaders {
-			break
+			return peer
 		}
 	}
-	return peer
+	return nil
 }
 
 // requests represents the peer specific pool of open requests
@@ -90,12 +89,12 @@ type requests struct {
 }
 
 type request struct {
-	hashes map[string]bool
-	c      chan []byte
-	cancel func()
+	hashes map[string]bool // remembers the block hashes that are requested in this connection
+	c      chan []byte     // channel in which the receoved block headers are passed on
+	cancel func()          // function to call in case of cancellation of the GetBlockHeaders event
 }
 
-// newRequestIDFunc is used to generated unique ID for requests
+// newRequestIDFunc is used to generate unique ID for requests
 // tests can reassign for deterministic ids
 var newRequestIDFunc = newRequestID
 
@@ -127,20 +126,20 @@ func (r *requests) create(c chan []byte) *request {
 
 func (r *requests) add(id uint32, req *request) {
 	r.mtx.Lock()
-	defer r.mtx.Unlock()
 	r.r[id] = req
+	r.mtx.Unlock()
 }
 
 func (r *requests) remove(id uint32) {
 	r.mtx.Lock()
-	defer r.mtx.Unlock()
 	delete(r.r, id)
+	r.mtx.Unlock()
 }
 
 func (r *requests) get(id uint32) (*request, bool) {
 	r.mtx.RLock()
-	defer r.mtx.RUnlock()
 	req, ok := r.r[id]
+	r.mtx.RUnlock()
 	return req, ok
 }
 

swarm.go

@@ -225,7 +225,7 @@ func NewSwarm(config *api.Config, mockStore *mock.NodeStore) (self *Swarm, err e
 
 	self.bzz = network.NewBzz(bzzconfig, to, self.stateStore, self.streamer.GetSpec(), self.streamer.Run)
 
-	self.bzzEth = bzzeth.New(self.netStore, to)
+	self.bzzEth = bzzeth.New()
 
 	// Pss = postal service over swarm (devp2p over bzz)
 	self.ps, err = pss.New(to, config.Pss)