Binary encoding of networkID (#205)

* networkIID - binary encoding
* refactor handshake tests

pkg/p2p/libp2p/internal/handshake/handshake.go

@@ -6,9 +6,9 @@ package handshake
 
 import (
 	"bytes"
+	"encoding/binary"
 	"errors"
 	"fmt"
-	"strconv"
 	"sync"
 	"time"
 
@@ -55,6 +55,7 @@ type Service struct {
 	signature            []byte
 	signer               crypto.Signer
 	networkID            uint64
+	networkIDBytes       []byte
 	receivedHandshakes   map[libp2ppeer.ID]struct{}
 	receivedHandshakesMu sync.Mutex
 	logger               logging.Logger
@@ -62,18 +63,26 @@ type Service struct {
 	network.Notifiee // handhsake service can be the receiver for network.Notify
 }
 
-func New(overlay swarm.Address, underlay string, signer crypto.Signer, networkID uint64, logger logging.Logger) (*Service, error) {
-	signature, err := signer.Sign([]byte(underlay + strconv.FormatUint(networkID, 10)))
+func New(overlay swarm.Address, peerID libp2ppeer.ID, signer crypto.Signer, networkID uint64, logger logging.Logger) (*Service, error) {
+	underlay, err := peerID.MarshalBinary()
+	if err != nil {
+		return nil, err
+	}
+
+	networkIDBytes := make([]byte, 8)
+	binary.BigEndian.PutUint64(networkIDBytes, networkID)
+	signature, err := signer.Sign(append(underlay, networkIDBytes...))
 	if err != nil {
 		return nil, err
 	}
 
 	return &Service{
 		overlay:            overlay,
-		underlay:           []byte(underlay),
+		underlay:           underlay,
 		signature:          signature,
 		signer:             signer,
 		networkID:          networkID,
+		networkIDBytes:     networkIDBytes,
 		receivedHandshakes: make(map[libp2ppeer.ID]struct{}),
 		logger:             logger,
 		Notifiee:           new(network.NoopNotifiee),
@@ -113,7 +122,6 @@ func (s *Service) Handshake(stream p2p.Stream) (i *Info, err error) {
 	}
 
 	s.logger.Tracef("handshake finished for peer %s", swarm.NewAddress(resp.Syn.BzzAddress.Overlay).String())
-
 	return &Info{
 		Overlay:   swarm.NewAddress(resp.Syn.BzzAddress.Overlay),
 		Underlay:  resp.Syn.BzzAddress.Underlay,
@@ -185,7 +193,7 @@ func (s *Service) checkSyn(syn *pb.Syn) error {
 		return ErrNetworkIDIncompatible
 	}
 
-	recoveredPK, err := crypto.Recover(syn.BzzAddress.Signature, append(syn.BzzAddress.Underlay, strconv.FormatUint(syn.NetworkID, 10)...))
+	recoveredPK, err := crypto.Recover(syn.BzzAddress.Signature, append(syn.BzzAddress.Underlay, s.networkIDBytes...))
 	if err != nil {
 		return ErrInvalidSignature
 	}

pkg/p2p/libp2p/internal/handshake/handshake_test.go

@@ -6,6 +6,7 @@ package handshake_test
 
 import (
 	"bytes"
+	"encoding/binary"
 	"errors"
 	"fmt"
 	"io/ioutil"
@@ -24,67 +25,90 @@ import (
 )
 
 func TestHandshake(t *testing.T) {
-	node1Underlay := []byte("underlay1")
-	node2Underlay := []byte("underlay2")
 	logger := logging.New(ioutil.Discard, 0)
-
-	privateKey1, err := crypto.GenerateSecp256k1Key()
+	node1ma, err := ma.NewMultiaddr("/ip4/127.0.0.1/tcp/7070/p2p/16Uiu2HAkx8ULY8cTXhdVAcMmLcH9AsTKz6uBQ7DPLKRjMLgBVYkA")
 	if err != nil {
 		t.Fatal(err)
 	}
-
-	privateKey2, err := crypto.GenerateSecp256k1Key()
+	node2ma, err := ma.NewMultiaddr("/ip4/127.0.0.1/tcp/7070/p2p/16Uiu2HAkx8ULY8cTXhdVAcMmLcH9AsTKz6uBQ7DPLKRjMLgBVYkS")
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	node1Overlay := crypto.NewOverlayAddress(privateKey1.PublicKey, 0)
-	node2Overlay := crypto.NewOverlayAddress(privateKey2.PublicKey, 0)
-	signer1 := crypto.NewDefaultSigner(privateKey1)
-	signer2 := crypto.NewDefaultSigner(privateKey2)
+	node1Addr, err := peer.AddrInfoFromP2pAddr(node1ma)
+	if err != nil {
+		t.Fatal(err)
+	}
+	node2Addr, err := peer.AddrInfoFromP2pAddr(node2ma)
+	if err != nil {
+		t.Fatal(err)
+	}
 
-	signature1, err := signer1.Sign([]byte("underlay10"))
+	node1Underlay, err := node1Addr.ID.MarshalBinary()
+	if err != nil {
+		t.Fatal(err)
+	}
+	node2Underlay, err := node2Addr.ID.MarshalBinary()
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	signature2, err := signer2.Sign([]byte("underlay20"))
+	privateKey1, err := crypto.GenerateSecp256k1Key()
+	if err != nil {
+		t.Fatal(err)
+	}
+	privateKey2, err := crypto.GenerateSecp256k1Key()
 	if err != nil {
 		t.Fatal(err)
 	}
 
+	node1Overlay := crypto.NewOverlayAddress(privateKey1.PublicKey, 3)
+	node2Overlay := crypto.NewOverlayAddress(privateKey2.PublicKey, 3)
 	node1Info := handshake.Info{
 		Overlay:   node1Overlay,
 		Underlay:  node1Underlay,
-		NetworkID: 0,
+		NetworkID: 3,
 		Light:     false,
 	}
 
-	node1BzzAddress := &pb.BzzAddress{
-		Overlay:   node1Info.Overlay.Bytes(),
-		Underlay:  node1Info.Underlay,
-		Signature: signature1,
-	}
-
 	node2Info := handshake.Info{
 		Overlay:   node2Overlay,
 		Underlay:  node2Underlay,
-		NetworkID: 0,
+		NetworkID: 3,
 		Light:     false,
 	}
 
+	signer1 := crypto.NewDefaultSigner(privateKey1)
+	signer2 := crypto.NewDefaultSigner(privateKey2)
+	networkIDBytes := make([]byte, 8)
+	binary.BigEndian.PutUint64(networkIDBytes, node1Info.NetworkID)
+	signature1, err := signer1.Sign(append(node1Underlay, networkIDBytes...))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	signature2, err := signer2.Sign(append(node2Underlay, networkIDBytes...))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	node1BzzAddress := &pb.BzzAddress{
+		Overlay:   node1Info.Overlay.Bytes(),
+		Underlay:  node1Info.Underlay,
+		Signature: signature1,
+	}
 	node2BzzAddress := &pb.BzzAddress{
 		Overlay:   node2Info.Overlay.Bytes(),
 		Underlay:  node2Info.Underlay,
 		Signature: signature2,
 	}
 
-	handshakeService, err := handshake.New(node1Info.Overlay, string(node1Info.Underlay), signer1, 0, logger)
+	handshakeService, err := handshake.New(node1Info.Overlay, node1Addr.ID, signer1, node1Info.NetworkID, logger)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	t.Run("OK", func(t *testing.T) {
+	t.Run("Handshake - OK", func(t *testing.T) {
 		var buffer1 bytes.Buffer
 		var buffer2 bytes.Buffer
 		stream1 := mock.NewStream(&buffer1, &buffer2)
@@ -113,7 +137,7 @@ func TestHandshake(t *testing.T) {
 		}
 	})
 
-	t.Run("ERROR - Syn write error", func(t *testing.T) {
+	t.Run("Handshake - Syn write error", func(t *testing.T) {
 		testErr := errors.New("test error")
 		expectedErr := fmt.Errorf("write syn message: %w", testErr)
 		stream := &mock.Stream{}
@@ -128,7 +152,7 @@ func TestHandshake(t *testing.T) {
 		}
 	})
 
-	t.Run("ERROR - Syn read error", func(t *testing.T) {
+	t.Run("Handshake - Syn read error", func(t *testing.T) {
 		testErr := errors.New("test error")
 		expectedErr := fmt.Errorf("read synack message: %w", testErr)
 		stream := mock.NewStream(nil, &bytes.Buffer{})
@@ -143,7 +167,7 @@ func TestHandshake(t *testing.T) {
 		}
 	})
 
-	t.Run("ERROR - ack write error", func(t *testing.T) {
+	t.Run("Handshake - ack write error", func(t *testing.T) {
 		testErr := errors.New("test error")
 		expectedErr := fmt.Errorf("write ack message: %w", testErr)
 		var buffer1 bytes.Buffer
@@ -174,7 +198,7 @@ func TestHandshake(t *testing.T) {
 		}
 	})
 
-	t.Run("ERROR - networkID mismatch", func(t *testing.T) {
+	t.Run("Handshake - networkID mismatch", func(t *testing.T) {
 		var buffer1 bytes.Buffer
 		var buffer2 bytes.Buffer
 		stream1 := mock.NewStream(&buffer1, &buffer2)
@@ -202,7 +226,7 @@ func TestHandshake(t *testing.T) {
 		}
 	})
 
-	t.Run("ERROR - invalid ack", func(t *testing.T) {
+	t.Run("Handshake - invalid ack", func(t *testing.T) {
 		var buffer1 bytes.Buffer
 		var buffer2 bytes.Buffer
 		stream1 := mock.NewStream(&buffer1, &buffer2)
@@ -230,7 +254,7 @@ func TestHandshake(t *testing.T) {
 		}
 	})
 
-	t.Run("ERROR - invalid signature", func(t *testing.T) {
+	t.Run("Handshake - invalid signature", func(t *testing.T) {
 		var buffer1 bytes.Buffer
 		var buffer2 bytes.Buffer
 		stream1 := mock.NewStream(&buffer1, &buffer2)
@@ -261,78 +285,12 @@ func TestHandshake(t *testing.T) {
 			t.Fatalf("expected %s, got %s", handshake.ErrInvalidSignature, err)
 		}
 	})
-}
-
-func TestHandle(t *testing.T) {
-	privateKey1, err := crypto.GenerateSecp256k1Key()
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	privateKey2, err := crypto.GenerateSecp256k1Key()
-	if err != nil {
-		t.Fatal(err)
-	}
 
-	node1Overlay := crypto.NewOverlayAddress(privateKey1.PublicKey, 0)
-	node2Overlay := crypto.NewOverlayAddress(privateKey2.PublicKey, 0)
-	node2ma, err := ma.NewMultiaddr("/ip4/127.0.0.1/tcp/7070/p2p/16Uiu2HAkx8ULY8cTXhdVAcMmLcH9AsTKz6uBQ7DPLKRjMLgBVYkS")
+	t.Run("Handle - OK", func(t *testing.T) {
+		handshakeService, err := handshake.New(node1Info.Overlay, node1Addr.ID, signer1, node1Info.NetworkID, logger)
 		if err != nil {
 			t.Fatal(err)
 		}
-
-	node2AddrInfo, err := peer.AddrInfoFromP2pAddr(node2ma)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	node1Underlay := []byte("underlay1")
-	node2Underlay := []byte("16Uiu2HAkx8ULY8cTXhdVAcMmLcH9AsTKz6uBQ7DPLKRjMLgBVYkS")
-	node1Info := handshake.Info{
-		Overlay:   node1Overlay,
-		Underlay:  node1Underlay,
-		NetworkID: 0,
-		Light:     false,
-	}
-
-	signer1 := crypto.NewDefaultSigner(privateKey1)
-	signer2 := crypto.NewDefaultSigner(privateKey2)
-	signature1, err := signer1.Sign([]byte("underlay10"))
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	signature2, err := signer2.Sign([]byte("16Uiu2HAkx8ULY8cTXhdVAcMmLcH9AsTKz6uBQ7DPLKRjMLgBVYkS0"))
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	node2Info := handshake.Info{
-		Overlay:   node2Overlay,
-		Underlay:  node2Underlay,
-		NetworkID: 0,
-		Light:     false,
-	}
-
-	node1BzzAddress := &pb.BzzAddress{
-		Overlay:   node1Info.Overlay.Bytes(),
-		Underlay:  node1Info.Underlay,
-		Signature: signature1,
-	}
-
-	node2BzzAddress := &pb.BzzAddress{
-		Overlay:   node2Info.Overlay.Bytes(),
-		Underlay:  node2Info.Underlay,
-		Signature: signature2,
-	}
-
-	logger := logging.New(ioutil.Discard, 0)
-	t.Run("OK", func(t *testing.T) {
-		handshakeService, err := handshake.New(node1Info.Overlay, string(node1Info.Underlay), signer1, 0, logger)
-		if err != nil {
-			t.Fatal(err)
-		}
-
 		var buffer1 bytes.Buffer
 		var buffer2 bytes.Buffer
 		stream1 := mock.NewStream(&buffer1, &buffer2)
@@ -351,7 +309,7 @@ func TestHandle(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		res, err := handshakeService.Handle(stream1, node2AddrInfo.ID)
+		res, err := handshakeService.Handle(stream1, node2Addr.ID)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -372,17 +330,16 @@ func TestHandle(t *testing.T) {
 		})
 	})
 
-	t.Run("ERROR - read error ", func(t *testing.T) {
-		handshakeService, err := handshake.New(node1Info.Overlay, string(node1Info.Underlay), signer1, 0, logger)
+	t.Run("Handle - read error ", func(t *testing.T) {
+		handshakeService, err := handshake.New(node1Info.Overlay, node1Addr.ID, signer1, node1Info.NetworkID, logger)
 		if err != nil {
 			t.Fatal(err)
 		}
-
 		testErr := errors.New("test error")
 		expectedErr := fmt.Errorf("read syn message: %w", testErr)
 		stream := &mock.Stream{}
 		stream.SetReadErr(testErr, 0)
-		res, err := handshakeService.Handle(stream, node2AddrInfo.ID)
+		res, err := handshakeService.Handle(stream, node2Addr.ID)
 		if err == nil || err.Error() != expectedErr.Error() {
 			t.Fatal("expected:", expectedErr, "got:", err)
 		}
@@ -392,12 +349,11 @@ func TestHandle(t *testing.T) {
 		}
 	})
 
-	t.Run("ERROR - write error ", func(t *testing.T) {
-		handshakeService, err := handshake.New(node1Info.Overlay, string(node1Info.Underlay), signer1, 0, logger)
+	t.Run("Handle - write error ", func(t *testing.T) {
+		handshakeService, err := handshake.New(node1Info.Overlay, node1Addr.ID, signer1, node1Info.NetworkID, logger)
 		if err != nil {
 			t.Fatal(err)
 		}
-
 		testErr := errors.New("test error")
 		expectedErr := fmt.Errorf("write synack message: %w", testErr)
 		var buffer bytes.Buffer
@@ -412,7 +368,7 @@ func TestHandle(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		res, err := handshakeService.Handle(stream, node2AddrInfo.ID)
+		res, err := handshakeService.Handle(stream, node2Addr.ID)
 		if err == nil || err.Error() != expectedErr.Error() {
 			t.Fatal("expected:", expectedErr, "got:", err)
 		}
@@ -422,12 +378,11 @@ func TestHandle(t *testing.T) {
 		}
 	})
 
-	t.Run("ERROR - ack read error ", func(t *testing.T) {
-		handshakeService, err := handshake.New(node1Info.Overlay, string(node1Info.Underlay), signer1, 0, logger)
+	t.Run("Handle - ack read error ", func(t *testing.T) {
+		handshakeService, err := handshake.New(node1Info.Overlay, node1Addr.ID, signer1, node1Info.NetworkID, logger)
 		if err != nil {
 			t.Fatal(err)
 		}
-
 		testErr := errors.New("test error")
 		expectedErr := fmt.Errorf("read ack message: %w", testErr)
 		var buffer1 bytes.Buffer
@@ -444,7 +399,7 @@ func TestHandle(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		res, err := handshakeService.Handle(stream1, node2AddrInfo.ID)
+		res, err := handshakeService.Handle(stream1, node2Addr.ID)
 		if err == nil || err.Error() != expectedErr.Error() {
 			t.Fatal("expected:", expectedErr, "got:", err)
 		}
@@ -454,12 +409,11 @@ func TestHandle(t *testing.T) {
 		}
 	})
 
-	t.Run("ERROR - networkID mismatch ", func(t *testing.T) {
-		handshakeService, err := handshake.New(node1Info.Overlay, string(node1Info.Underlay), signer1, 0, logger)
+	t.Run("Handle - networkID mismatch ", func(t *testing.T) {
+		handshakeService, err := handshake.New(node1Info.Overlay, node1Addr.ID, signer1, node1Info.NetworkID, logger)
 		if err != nil {
 			t.Fatal(err)
 		}
-
 		var buffer1 bytes.Buffer
 		var buffer2 bytes.Buffer
 		stream1 := mock.NewStream(&buffer1, &buffer2)
@@ -474,7 +428,7 @@ func TestHandle(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		res, err := handshakeService.Handle(stream1, node2AddrInfo.ID)
+		res, err := handshakeService.Handle(stream1, node2Addr.ID)
 		if res != nil {
 			t.Fatal("res should be nil")
 		}
@@ -484,12 +438,11 @@ func TestHandle(t *testing.T) {
 		}
 	})
 
-	t.Run("ERROR - duplicate handshake", func(t *testing.T) {
-		handshakeService, err := handshake.New(node1Info.Overlay, string(node1Info.Underlay), signer1, 0, logger)
+	t.Run("Handle - duplicate handshake", func(t *testing.T) {
+		handshakeService, err := handshake.New(node1Info.Overlay, node1Addr.ID, signer1, node1Info.NetworkID, logger)
 		if err != nil {
 			t.Fatal(err)
 		}
-
 		var buffer1 bytes.Buffer
 		var buffer2 bytes.Buffer
 		stream1 := mock.NewStream(&buffer1, &buffer2)
@@ -508,7 +461,7 @@ func TestHandle(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		res, err := handshakeService.Handle(stream1, node2AddrInfo.ID)
+		res, err := handshakeService.Handle(stream1, node2Addr.ID)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -528,18 +481,17 @@ func TestHandle(t *testing.T) {
 			Light:     got.Syn.Light,
 		})
 
-		_, err = handshakeService.Handle(stream1, node2AddrInfo.ID)
+		_, err = handshakeService.Handle(stream1, node2Addr.ID)
 		if err != handshake.ErrHandshakeDuplicate {
 			t.Fatalf("expected %s, got %s", handshake.ErrHandshakeDuplicate, err)
 		}
 	})
 
-	t.Run("Error - invalid ack", func(t *testing.T) {
-		handshakeService, err := handshake.New(node1Info.Overlay, string(node1Info.Underlay), signer1, 0, logger)
+	t.Run("Handle - invalid ack", func(t *testing.T) {
+		handshakeService, err := handshake.New(node1Info.Overlay, node1Addr.ID, signer1, node1Info.NetworkID, logger)
 		if err != nil {
 			t.Fatal(err)
 		}
-
 		var buffer1 bytes.Buffer
 		var buffer2 bytes.Buffer
 		stream1 := mock.NewStream(&buffer1, &buffer2)
@@ -558,18 +510,17 @@ func TestHandle(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		_, err = handshakeService.Handle(stream1, node2AddrInfo.ID)
+		_, err = handshakeService.Handle(stream1, node2Addr.ID)
 		if err != handshake.ErrInvalidAck {
 			t.Fatalf("expected %s, got %s", handshake.ErrInvalidAck, err)
 		}
 	})
 
-	t.Run("ERROR - invalid signature ", func(t *testing.T) {
-		handshakeService, err := handshake.New(node1Info.Overlay, string(node1Info.Underlay), signer1, 0, logger)
+	t.Run("Handle - invalid signature ", func(t *testing.T) {
+		handshakeService, err := handshake.New(node1Info.Overlay, node1Addr.ID, signer1, node1Info.NetworkID, logger)
 		if err != nil {
 			t.Fatal(err)
 		}
-
 		var buffer1 bytes.Buffer
 		var buffer2 bytes.Buffer
 		stream1 := mock.NewStream(&buffer1, &buffer2)
@@ -588,7 +539,7 @@ func TestHandle(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		res, err := handshakeService.Handle(stream1, node2AddrInfo.ID)
+		res, err := handshakeService.Handle(stream1, node2Addr.ID)
 		if res != nil {
 			t.Fatal("res should be nil")
 		}
@@ -602,7 +553,6 @@ func TestHandle(t *testing.T) {
 // testInfo validates if two Info instances are equal.
 func testInfo(t *testing.T, got, want handshake.Info) {
 	t.Helper()
-
 	if !got.Overlay.Equal(want.Overlay) || !bytes.Equal(got.Underlay, want.Underlay) || got.NetworkID != want.NetworkID || got.Light != want.Light {
 		t.Fatalf("got info %+v, want %+v", got, want)
 	}

pkg/p2p/libp2p/libp2p.go

@@ -47,7 +47,7 @@ type Service struct {
 	metrics          metrics
 	networkID        uint64
 	handshakeService *handshake.Service
-	addrssbook       addressbook.Putter
+	addressbook      addressbook.Putter
 	peers            *peerRegistry
 	peerHandler      func(context.Context, swarm.Address) error
 	conectionBreaker breaker.Interface
@@ -154,7 +154,7 @@ func New(ctx context.Context, signer beecrypto.Signer, networkID uint64, overlay
 		return nil, fmt.Errorf("autonat: %w", err)
 	}
 
-	handshakeService, err := handshake.New(overlay, h.ID().String(), signer, networkID, o.Logger)
+	handshakeService, err := handshake.New(overlay, h.ID(), signer, networkID, o.Logger)
 	if err != nil {
 		return nil, fmt.Errorf("handshake service: %w", err)
 	}
@@ -168,7 +168,7 @@ func New(ctx context.Context, signer beecrypto.Signer, networkID uint64, overlay
 		networkID:        networkID,
 		handshakeService: handshakeService,
 		peers:            peerRegistry,
-		addrssbook:       o.Addressbook,
+		addressbook:      o.Addressbook,
 		logger:           o.Logger,
 		tracer:           o.Tracer,
 		conectionBreaker: breaker.NewBreaker(breaker.Options{}), // todo: fill non-default options
@@ -206,7 +206,7 @@ func New(ctx context.Context, signer beecrypto.Signer, networkID uint64, overlay
 			return
 		}
 
-		err = s.addrssbook.Put(i.Overlay, remoteMultiaddr)
+		err = s.addressbook.Put(i.Overlay, remoteMultiaddr)
 		if err != nil {
 			s.logger.Debugf("handshake: addressbook put error %s: %v", peerID, err)
 			s.logger.Errorf("unable to persist peer %v", peerID)