Skip to content
This repository has been archived by the owner on Aug 2, 2021. It is now read-only.

all:bzzaddr instead of just overlay for logging #2000

Merged
merged 18 commits into from
Dec 4, 2019
Merged
20 changes: 11 additions & 9 deletions api/inspector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package api

import (
"crypto/rand"
"encoding/hex"
"io/ioutil"
"os"
"strings"
Expand All @@ -13,7 +12,6 @@ import (
"github.com/ethersphere/swarm/storage"
"github.com/ethersphere/swarm/storage/localstore"

"github.com/ethereum/go-ethereum/p2p/enode"
"github.com/ethereum/go-ethereum/rpc"
"github.com/ethersphere/swarm/state"
)
Expand All @@ -33,16 +31,18 @@ func TestInspectorPeerStreams(t *testing.T) {
t.Fatal(err)
}

// using the same key in for underlay address as well as it is not important for test
baseAddress := network.NewBzzAddr(baseKey, baseKey)
Copy link
Member

Choose a reason for hiding this comment

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

Just an idea, for the underlay address, to use nil instead, not to present an example where overlay address is used as a value from underlay?

The same question stands for other places where NewBzzAddr is used with identical value for arguments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, I started to change this on the whole project but did not push yet :) Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, this concrete case needs underlay address because of the PeerInfo() function, so I have decided to create another rand key for peer base address. I know it's not needed, but I think it gives more info in tests about the nature of address. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Or maybe just to add a comment justifying the usage of baseKey in this particular case. That would be also good, at least for me.

localStore, err := localstore.New(dir, baseKey, &localstore.Options{})
if err != nil {
t.Fatal(err)
}
netStore := storage.NewNetStore(localStore, baseKey, enode.ID{})
netStore := storage.NewNetStore(localStore, baseAddress)

i := NewInspector(nil, nil, netStore, stream.New(state.NewInmemoryStore(), baseKey, stream.NewSyncProvider(netStore, network.NewKademlia(
i := NewInspector(nil, nil, netStore, stream.New(state.NewInmemoryStore(), baseAddress, stream.NewSyncProvider(netStore, network.NewKademlia(
baseKey,
network.NewKadParams(),
), false, false)), localStore)
), baseAddress, false, false)), localStore)

server := rpc.NewServer()
if err := server.RegisterName("inspector", i); err != nil {
Expand All @@ -58,7 +58,7 @@ func TestInspectorPeerStreams(t *testing.T) {
t.Fatal(err)
}

if !strings.Contains(peerInfo, `"base":"`+hex.EncodeToString(baseKey)[:16]+`"`) {
if !strings.Contains(peerInfo, `"base":"`+baseAddress.ShortUnder()) {
t.Error("missing base key in response")
}
}
Expand All @@ -77,16 +77,18 @@ func TestInspectorStorageIndices(t *testing.T) {
t.Fatal(err)
}

// using the same key in for underlay address as well as it is not important for test
baseAddress := network.NewBzzAddr(baseKey, baseKey)
localStore, err := localstore.New(dir, baseKey, &localstore.Options{})
if err != nil {
t.Fatal(err)
}
netStore := storage.NewNetStore(localStore, baseKey, enode.ID{})
netStore := storage.NewNetStore(localStore, baseAddress)

i := NewInspector(nil, nil, netStore, stream.New(state.NewInmemoryStore(), baseKey, stream.NewSyncProvider(netStore, network.NewKademlia(
i := NewInspector(nil, nil, netStore, stream.New(state.NewInmemoryStore(), network.NewBzzAddr(baseKey, baseKey), stream.NewSyncProvider(netStore, network.NewKademlia(
baseKey,
network.NewKadParams(),
), false, false)), localStore)
), baseAddress, false, false)), localStore)

server := rpc.NewServer()
if err := server.RegisterName("inspector", i); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions bzzeth/bzzeth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ func newTestNetworkStore(t *testing.T) (prvkey *ecdsa.PrivateKey, netStore *stor
t.Fatalf("Could not create localStore")
}

netStore = storage.NewNetStore(localStore, bzzAddr, enode.ID{})
r := retrieval.New(kad, netStore, bzzAddr, nil)
netStore = storage.NewNetStore(localStore, network.NewBzzAddr(bzzAddr, nil))
r := retrieval.New(kad, netStore, network.NewBzzAddr(bzzAddr, nil), nil)
netStore.RemoteGet = r.RequestFromPeers

cleanup = func() {
Expand Down
5 changes: 2 additions & 3 deletions cmd/swarm-snapshot/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,8 @@ func createSnapshot(filename string, nodes int, services []string) (err error) {
bucket.Store(simulation.BucketKeyKademlia, kad)

config := &network.BzzConfig{
OverlayAddr: addr.Over(),
UnderlayAddr: addr.Under(),
HiveParams: hp,
Address: addr,
HiveParams: hp,
}
return network.NewBzz(config, kad, nil, nil, nil, nil, nil), nil, nil
},
Expand Down
2 changes: 1 addition & 1 deletion network/kademlia_load_balancer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ func TestResourceUseStats(t *testing.T) {
for i := uint64(0); i < 10; i++ {
a := make([]byte, 8)
binary.BigEndian.PutUint64(a, i)
p := NewPeer(&BzzPeer{BzzAddr: NewBzzAddr(a, a)}, nil)
p := NewPeer(&BzzPeer{BzzAddr: NewBzzAddr(a, nil)}, nil)
k.On(p)
if delay > 0 {
time.Sleep(delay)
Expand Down
2 changes: 1 addition & 1 deletion network/kademlia_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func init() {

func testKadPeerAddr(s string) *BzzAddr {
a := pot.NewAddressFromString(s)
return NewBzzAddr(a, a)
return NewBzzAddr(a, nil)
}

func newTestKademliaParams() *KadParams {
Expand Down
23 changes: 23 additions & 0 deletions network/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package network

import (
"crypto/ecdsa"
"encoding/hex"
"fmt"
"io"
"net"
Expand Down Expand Up @@ -90,6 +91,28 @@ func (a *BzzAddr) Under() []byte {
return a.UAddr
}

// ShortString prints beginning of the OAddr and UAddr
// It can be used for id in logging
func (a *BzzAddr) ShortString() string {
return fmt.Sprintf("%s:%s", a.ShortOver(), a.ShortUnder())
}

func (a *BzzAddr) ShortOver() string {
Copy link
Member

Choose a reason for hiding this comment

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

comment exported functions

if oaddr := hex.EncodeToString(a.OAddr); len(oaddr) >= 16 {
return oaddr[:16]
} else {
return oaddr
}
}

func (a *BzzAddr) ShortUnder() string {
if uaddr := hex.EncodeToString(a.UAddr); len(uaddr) >= 16 {
return uaddr[:16]
} else {
return uaddr
}
}

// ID returns the node identifier in the underlay.
func (a *BzzAddr) ID() enode.ID {
n, err := enode.ParseV4(string(a.UAddr))
Expand Down
7 changes: 3 additions & 4 deletions network/networkid_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,10 +211,9 @@ func newServices() adapters.Services {
nodeMap[currentNetworkID] = append(nodeMap[currentNetworkID], ctx.Config.ID)
log.Debug("current network ID:", "id", currentNetworkID)
config := &BzzConfig{
OverlayAddr: addr.Over(),
UnderlayAddr: addr.Under(),
HiveParams: hp,
NetworkID: uint64(currentNetworkID),
Address: addr,
HiveParams: hp,
NetworkID: uint64(currentNetworkID),
}
return NewBzz(config, kademlia(ctx.Config.ID), nil, nil, nil, nil, nil), nil
},
Expand Down
6 changes: 3 additions & 3 deletions network/protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,7 @@ func isFullCapability(c *capability.Capability) bool {

// BzzConfig captures the config params used by the hive
type BzzConfig struct {
OverlayAddr []byte // base address of the overlay network
UnderlayAddr []byte // node's underlay address
Address *BzzAddr
HiveParams *HiveParams
NetworkID uint64
LightNode bool // temporarily kept as we still only define light/full on operational level
Expand Down Expand Up @@ -140,7 +139,7 @@ func NewBzz(config *BzzConfig, kad *Kademlia, store state.Store, streamerSpec, r
bzz := &Bzz{
Hive: NewHive(config.HiveParams, kad, store),
NetworkID: config.NetworkID,
localAddr: NewBzzAddr(config.OverlayAddr, config.UnderlayAddr),
localAddr: config.Address,
handshakes: make(map[enode.ID]*HandshakeMsg),
streamerRun: streamerRun,
streamerSpec: streamerSpec,
Expand Down Expand Up @@ -338,6 +337,7 @@ type BzzPeer struct {
}

func NewBzzPeer(p *protocols.Peer) *BzzPeer {
// TODO The overlay address is not correct until there is a way to fetch overlay address from the peer
return &BzzPeer{Peer: p, BzzAddr: NewBzzAddrFromEnode(p.Node())}
}

Expand Down
9 changes: 4 additions & 5 deletions network/protocol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,11 +169,10 @@ type bzzTester struct {

func newBzz(addr *BzzAddr, lightNode bool) *Bzz {
config := &BzzConfig{
OverlayAddr: addr.Over(),
UnderlayAddr: addr.Under(),
HiveParams: NewHiveParams(),
NetworkID: DefaultTestNetworkID,
LightNode: lightNode,
Address: addr,
HiveParams: NewHiveParams(),
NetworkID: DefaultTestNetworkID,
LightNode: lightNode,
}
kad := NewKademlia(addr.OAddr, NewKadParams())
bzz := NewBzz(config, kad, nil, nil, nil, nil, nil)
Expand Down
5 changes: 2 additions & 3 deletions network/retrieval/peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package retrieval

import (
"bytes"
"encoding/hex"
"errors"
"sync"

Expand All @@ -38,10 +37,10 @@ type Peer struct {
}

// NewPeer is the constructor for Peer
func NewPeer(peer *network.BzzPeer, baseKey []byte) *Peer {
func NewPeer(peer *network.BzzPeer, baseKey *network.BzzAddr) *Peer {
return &Peer{
BzzPeer: peer,
logger: log.New("base", hex.EncodeToString(baseKey)[:16], "peer", peer.ID().String()[:16]),
logger: log.New("base", baseKey.ShortString(), "peer", peer.BzzAddr.ShortString()),
retrievals: make(map[uint]chunk.Address),
}
}
Expand Down
35 changes: 18 additions & 17 deletions network/retrieval/retrieve.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package retrieval
import (
"bytes"
"context"
"encoding/hex"
"errors"
"fmt"
"math/rand"
Expand Down Expand Up @@ -93,24 +92,26 @@ func (cd *ChunkDelivery) Price() *protocols.Price {

// Retrieval holds state and handles protocol messages for the `bzz-retrieve` protocol
type Retrieval struct {
netStore *storage.NetStore
kad *network.Kademlia
mtx sync.RWMutex // protect peer map
peers map[enode.ID]*Peer // compatible peers
spec *protocols.Spec // protocol spec
logger log.Logger // custom logger to append a basekey
quit chan struct{} // shutdown channel
netStore *storage.NetStore
baseAddress *network.BzzAddr
kad *network.Kademlia
mtx sync.RWMutex // protect peer map
peers map[enode.ID]*Peer // compatible peers
spec *protocols.Spec // protocol spec
logger log.Logger // custom logger to append a basekey
quit chan struct{} // shutdown channel
}

// New returns a new instance of the retrieval protocol handler
func New(kad *network.Kademlia, ns *storage.NetStore, baseKey []byte, balance protocols.Balance) *Retrieval {
func New(kad *network.Kademlia, ns *storage.NetStore, baseKey *network.BzzAddr, balance protocols.Balance) *Retrieval {
r := &Retrieval{
netStore: ns,
kad: kad,
peers: make(map[enode.ID]*Peer),
spec: spec,
logger: log.New("base", hex.EncodeToString(baseKey)[:16]),
quit: make(chan struct{}),
netStore: ns,
kad: kad,
peers: make(map[enode.ID]*Peer),
spec: spec,
logger: log.New("base", baseKey.ShortString()),
baseAddress: baseKey,
quit: make(chan struct{}),
}
if balance != nil && !reflect.ValueOf(balance).IsNil() {
// swap is enabled, so setup the hook
Expand Down Expand Up @@ -142,7 +143,7 @@ func (r *Retrieval) getPeer(id enode.ID) *Peer {

// Run is being dispatched when 2 nodes connect
func (r *Retrieval) Run(bp *network.BzzPeer) error {
sp := NewPeer(bp, r.kad.BaseAddr())
sp := NewPeer(bp, r.baseAddress)
r.addPeer(sp)
defer r.removePeer(sp)

Expand Down Expand Up @@ -227,7 +228,7 @@ func (r *Retrieval) findPeer(ctx context.Context, req *storage.Request) (retPeer

// skip peers that we have already tried
if req.SkipPeer(id.String()) {
r.logger.Trace("findpeer skip peer", "peer", id, "ref", req.Addr.String())
r.logger.Trace("findpeer skip peer", "peer", p.ShortString(), "ref", req.Addr.String())
return true
}

Expand Down
10 changes: 5 additions & 5 deletions network/retrieval/retrieve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ func TestRequestFromPeers(t *testing.T) {

to.On(peer)

s := New(to, nil, to.BaseAddr(), nil)
s := New(to, nil, addr, nil)

req := storage.NewRequest(storage.Address(hash0[:]))
id, err := s.findPeer(context.Background(), req)
Expand Down Expand Up @@ -487,7 +487,7 @@ func newBzzRetrieveWithLocalstore(ctx *adapters.ServiceContext, bucket *sync.Map
k, _ := bucket.LoadOrStore(simulation.BucketKeyKademlia, network.NewKademlia(addr.Over(), network.NewKadParams()))
kad := k.(*network.Kademlia)

netStore := storage.NewNetStore(localStore, kad.BaseAddr(), n.ID())
netStore := storage.NewNetStore(localStore, addr)
lnetStore := storage.NewLNetStore(netStore)
fileStore := storage.NewFileStore(lnetStore, lnetStore, storage.NewFileStoreParams(), chunk.NewTags())

Expand All @@ -502,7 +502,7 @@ func newBzzRetrieveWithLocalstore(ctx *adapters.ServiceContext, bucket *sync.Map
return nil, nil, err
}

r := New(kad, netStore, kad.BaseAddr(), nil)
r := New(kad, netStore, addr, nil)
netStore.RemoteGet = r.RequestFromPeers
bucket.Store(bucketKeyFileStore, fileStore)
bucket.Store(bucketKeyNetstore, netStore)
Expand Down Expand Up @@ -621,7 +621,7 @@ func newRetrievalTester(t *testing.T, prvkey *ecdsa.PrivateKey, netStore *storag
prvkey = key
}

r := New(kad, netStore, kad.BaseAddr(), nil)
r := New(kad, netStore, network.NewBzzAddr(kad.BaseAddr(), nil), nil)
protocolTester := p2ptest.NewProtocolTester(prvkey, 1, r.runProtocol)

return protocolTester, r, protocolTester.Stop, nil
Expand All @@ -646,7 +646,7 @@ func newTestNetstore(t *testing.T) (prvkey *ecdsa.PrivateKey, netStore *storage.
t.Fatalf("Could not create localStore")
}

netStore = storage.NewNetStore(localStore, bzzAddr, enode.ID{})
netStore = storage.NewNetStore(localStore, network.NewBzzAddr(bzzAddr, nil))

cleanup = func() {
err = netStore.Close()
Expand Down
5 changes: 2 additions & 3 deletions network/simulation/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,8 @@ func ExampleSimulation_WaitTillHealthy() {
hp := network.NewHiveParams()
hp.Discovery = false
config := &network.BzzConfig{
OverlayAddr: addr.Over(),
UnderlayAddr: addr.Under(),
HiveParams: hp,
Address: addr,
HiveParams: hp,
}
kad := network.NewKademlia(addr.Over(), network.NewKadParams())
// store kademlia in node's bucket under BucketKeyKademlia
Expand Down
5 changes: 2 additions & 3 deletions network/simulation/kademlia_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,8 @@ func createSimServiceMap(discovery bool) map[string]ServiceFunc {
hp := network.NewHiveParams()
hp.Discovery = discovery
config := &network.BzzConfig{
OverlayAddr: addr.Over(),
UnderlayAddr: addr.Under(),
HiveParams: hp,
Address: addr,
HiveParams: hp,
}
kad := network.NewKademlia(addr.Over(), network.NewKadParams())
// store kademlia in node's bucket under BucketKeyKademlia
Expand Down
5 changes: 2 additions & 3 deletions network/simulation/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,9 +284,8 @@ func TestUploadSnapshot(t *testing.T) {
hp := network.NewHiveParams()
hp.Discovery = false
config := &network.BzzConfig{
OverlayAddr: addr.Over(),
UnderlayAddr: addr.Under(),
HiveParams: hp,
Address: addr,
HiveParams: hp,
}
kad := network.NewKademlia(addr.Over(), network.NewKadParams())
b.Store(BucketKeyKademlia, kad)
Expand Down
5 changes: 2 additions & 3 deletions network/simulation/simulation.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,8 @@ func NewBzzInProc(services map[string]ServiceFunc, disableAutoConnect bool) (s *
kad := k.(*network.Kademlia)

config := &network.BzzConfig{
OverlayAddr: addr.Over(),
UnderlayAddr: addr.Under(),
HiveParams: hp,
Address: addr,
HiveParams: hp,
}
return network.NewBzz(config, kad, nil, nil, nil, nil, nil), nil, nil
}
Expand Down
5 changes: 2 additions & 3 deletions network/simulations/discovery/discovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -509,9 +509,8 @@ func newService(ctx *adapters.ServiceContext) (node.Service, error) {
log.Info(fmt.Sprintf("discovery for nodeID %s is %t", ctx.Config.ID.String(), hp.Discovery))

config := &network.BzzConfig{
OverlayAddr: addr.Over(),
UnderlayAddr: addr.Under(),
HiveParams: hp,
Address: addr,
HiveParams: hp,
}

if persistenceEnabled {
Expand Down
Loading