refactor: Merge node into net and improve coverage (#1593)

## Relevant issue(s) Resolves #906 Resolves #1123 Resolves #1127 Resolves #1063 ## Description This PR does a refactor of the `net` package by merging into it the `node` package. There is a considerable improvement of the test coverage and `github.com/gogo/protobuf` is replaced with `github.com/planetscale/vtprotobuf`.
sourcenetwork · Jul 11, 2023 · 80c22b2 · 80c22b2
1 parent 5e35e00
commit 80c22b2
Show file tree

Hide file tree

Showing 38 changed files with 13,104 additions and 5,810 deletions.
diff --git a/cli/start.go b/cli/start.go
@@ -36,10 +36,9 @@ import (
 	"github.com/sourcenetwork/defradb/db"
 	"github.com/sourcenetwork/defradb/errors"
 	"github.com/sourcenetwork/defradb/logging"
-	netapi "github.com/sourcenetwork/defradb/net/api"
-	netpb "github.com/sourcenetwork/defradb/net/api/pb"
+	"github.com/sourcenetwork/defradb/net"
+	netpb "github.com/sourcenetwork/defradb/net/pb"
 	netutils "github.com/sourcenetwork/defradb/net/utils"
-	"github.com/sourcenetwork/defradb/node"
 )
 
 func MakeStartCommand(cfg *config.Config) *cobra.Command {
@@ -194,7 +193,7 @@ func MakeStartCommand(cfg *config.Config) *cobra.Command {
 }
 
 type defraInstance struct {
-	node   *node.Node
+	node   *net.Node
 	db     client.DB
 	server *httpapi.Server
 }
@@ -252,13 +251,13 @@ func start(ctx context.Context, cfg *config.Config) (*defraInstance, error) {
 	}
 
 	// init the p2p node
-	var n *node.Node
+	var n *net.Node
 	if !cfg.Net.P2PDisabled {
 		log.FeedbackInfo(ctx, "Starting P2P node", logging.NewKV("P2P address", cfg.Net.P2PAddress))
-		n, err = node.NewNode(
+		n, err = net.NewNode(
 			ctx,
 			db,
-			cfg.NodeConfig(),
+			net.WithConfig(cfg),
 		)
 		if err != nil {
 			db.Close(ctx)
@@ -315,11 +314,9 @@ func start(ctx context.Context, cfg *config.Config) (*defraInstance, error) {
 			return nil, errors.Wrap(fmt.Sprintf("failed to listen on TCP address %v", addr), err)
 		}
 
-		netService := netapi.NewService(n.Peer)
-
 		go func() {
 			log.FeedbackInfo(ctx, "Started RPC server", logging.NewKV("Address", addr))
-			netpb.RegisterServiceServer(server, netService)
+			netpb.RegisterCollectionServer(server, n.Peer)
 			if err := server.Serve(tcplistener); err != nil && !errors.Is(err, grpc.ErrServerStopped) {
 				log.FeedbackFatalE(ctx, "Failed to start RPC server", err)
 			}

diff --git a/config/config.go b/config/config.go
@@ -61,7 +61,6 @@ import (
 
 	badgerds "github.com/sourcenetwork/defradb/datastore/badger/v3"
 	"github.com/sourcenetwork/defradb/logging"
-	"github.com/sourcenetwork/defradb/node"
 )
 
 var log = logging.MustNewLogger("config")
@@ -424,29 +423,6 @@ func (netcfg *NetConfig) RPCMaxConnectionIdleDuration() (time.Duration, error) {
 	return d, nil
 }
 
-// NodeConfig provides the Node-specific configuration, from the top-level Net config.
-func (cfg *Config) NodeConfig() node.NodeOpt {
-	return func(opt *node.Options) error {
-		var err error
-		err = node.ListenP2PAddrStrings(cfg.Net.P2PAddress)(opt)
-		if err != nil {
-			return err
-		}
-		err = node.ListenTCPAddrString(cfg.Net.TCPAddress)(opt)
-		if err != nil {
-			return err
-		}
-		opt.EnableRelay = cfg.Net.RelayEnabled
-		opt.EnablePubSub = cfg.Net.PubSubEnabled
-		opt.DataPath = cfg.Datastore.Badger.Path
-		opt.ConnManager, err = node.NewConnManager(100, 400, time.Second*20)
-		if err != nil {
-			return err
-		}
-		return nil
-	}
-}
-
 // LogConfig configures output and logger.
 type LoggingConfig struct {
 	Level          string

diff --git a/config/config_test.go b/config/config_test.go
@@ -17,10 +17,7 @@ import (
 	"testing"
 	"time"
 
-	ma "github.com/multiformats/go-multiaddr"
 	"github.com/stretchr/testify/assert"
-
-	"github.com/sourcenetwork/defradb/node"
 )
 
 var envVarsDifferent = map[string]string{
@@ -224,47 +221,6 @@ func TestInvalidEnvVars(t *testing.T) {
 	assert.ErrorIs(t, err, ErrLoadingConfig)
 }
 
-func TestNodeConfig(t *testing.T) {
-	cfg := DefaultConfig()
-	cfg.Net.P2PAddress = "/ip4/0.0.0.0/tcp/9179"
-	cfg.Net.TCPAddress = "/ip4/0.0.0.0/tcp/9169"
-	cfg.Net.RPCTimeout = "100s"
-	cfg.Net.RPCMaxConnectionIdle = "111s"
-	cfg.Net.RelayEnabled = true
-	cfg.Net.PubSubEnabled = true
-	cfg.Datastore.Badger.Path = "/tmp/defra_cli/badger"
-
-	err := cfg.validate()
-	assert.NoError(t, err)
-
-	nodeConfig := cfg.NodeConfig()
-	options, errOptionsMerge := node.NewMergedOptions(nodeConfig)
-
-	// confirming it provides the same config as a manually constructed node.Options
-	p2pAddr, errP2P := ma.NewMultiaddr(cfg.Net.P2PAddress)
-	tcpAddr, errTCP := ma.NewMultiaddr(cfg.Net.TCPAddress)
-	connManager, errConnManager := node.NewConnManager(100, 400, time.Second*20)
-	expectedOptions := node.Options{
-		ListenAddrs:  []ma.Multiaddr{p2pAddr},
-		TCPAddr:      tcpAddr,
-		DataPath:     "/tmp/defra_cli/badger",
-		EnablePubSub: true,
-		EnableRelay:  true,
-		ConnManager:  connManager,
-	}
-	assert.NoError(t, errOptionsMerge)
-	assert.NoError(t, errP2P)
-	assert.NoError(t, errTCP)
-	assert.NoError(t, errConnManager)
-	for k, v := range options.ListenAddrs {
-		assert.Equal(t, expectedOptions.ListenAddrs[k], v)
-	}
-	assert.Equal(t, expectedOptions.TCPAddr.String(), options.TCPAddr.String())
-	assert.Equal(t, expectedOptions.DataPath, options.DataPath)
-	assert.Equal(t, expectedOptions.EnablePubSub, options.EnablePubSub)
-	assert.Equal(t, expectedOptions.EnableRelay, options.EnableRelay)
-}
-
 func TestCreateAndLoadCustomConfig(t *testing.T) {
 	testdir := t.TempDir()
 

diff --git a/go.mod b/go.mod
@@ -12,7 +12,6 @@ require (
 	github.com/go-chi/cors v1.2.1
 	github.com/go-errors/errors v1.4.2
 	github.com/gofrs/uuid/v5 v5.0.0
-	github.com/gogo/protobuf v1.3.2
 	github.com/graphql-go/graphql v0.8.1
 	github.com/grpc-ecosystem/go-grpc-middleware v1.4.0
 	github.com/iancoleman/strcase v0.2.0
@@ -33,8 +32,8 @@ require (
 	github.com/multiformats/go-multiaddr v0.10.0
 	github.com/multiformats/go-multibase v0.2.0
 	github.com/multiformats/go-multihash v0.2.3
-	github.com/multiformats/go-varint v0.0.7
 	github.com/pkg/errors v0.9.1
+	github.com/planetscale/vtprotobuf v0.4.0
 	github.com/sourcenetwork/immutable v0.2.2
 	github.com/spf13/cobra v1.7.0
 	github.com/spf13/pflag v1.0.5
@@ -50,6 +49,7 @@ require (
 	golang.org/x/crypto v0.11.0
 	golang.org/x/net v0.12.0
 	google.golang.org/grpc v1.56.2
+	google.golang.org/protobuf v1.30.0
 )
 
 require (
@@ -75,6 +75,7 @@ require (
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect
 	github.com/godbus/dbus/v5 v5.1.0 // indirect
+	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang/glog v1.1.0 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/mock v1.6.0 // indirect
@@ -146,6 +147,7 @@ require (
 	github.com/multiformats/go-multiaddr-fmt v0.1.0 // indirect
 	github.com/multiformats/go-multicodec v0.9.0 // indirect
 	github.com/multiformats/go-multistream v0.4.1 // indirect
+	github.com/multiformats/go-varint v0.0.7 // indirect
 	github.com/onsi/ginkgo/v2 v2.9.7 // indirect
 	github.com/opencontainers/runtime-spec v1.0.2 // indirect
 	github.com/opentracing/opentracing-go v1.2.0 // indirect
@@ -191,7 +193,6 @@ require (
 	golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
 	gonum.org/v1/gonum v0.13.0 // indirect
 	google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1 // indirect
-	google.golang.org/protobuf v1.30.0 // indirect
 	gopkg.in/ini.v1 v1.67.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	lukechampine.com/blake3 v1.2.1 // indirect

diff --git a/go.sum b/go.sum
@@ -1082,6 +1082,8 @@ github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pkg/profile v1.2.1/go.mod h1:hJw3o1OdXxsrSjjVksARp5W95eeEaEfptyVZyv6JUPA=
 github.com/pkg/sftp v1.13.1/go.mod h1:3HaPG6Dq1ILlpPZRO0HVMrsydcdLt6HRDccSgb87qRg=
+github.com/planetscale/vtprotobuf v0.4.0 h1:NEI+g4woRaAZgeZ3sAvbtyvMBRjIv5kE7EWYQ8m4JwY=
+github.com/planetscale/vtprotobuf v0.4.0/go.mod h1:wm1N3qk9G/4+VM1WhpkLbvY/d8+0PbwYYpP5P5VhTks=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/polydawn/refmt v0.0.0-20190221155625-df39d6c2d992/go.mod h1:uIp+gprXxxrWSjjklXD+mN4wed/tMfjMMmN/9+JsA9o=

diff --git a/net/api/client/client.go b/net/api/client/client.go
@@ -18,15 +18,22 @@ import (
 
 	"github.com/libp2p/go-libp2p/core/peer"
 	ma "github.com/multiformats/go-multiaddr"
+	codec "github.com/planetscale/vtprotobuf/codec/grpc"
 	"google.golang.org/grpc"
+	"google.golang.org/grpc/encoding"
+	_ "google.golang.org/grpc/encoding/proto"
 
 	"github.com/sourcenetwork/defradb/client"
 	"github.com/sourcenetwork/defradb/errors"
-	pb "github.com/sourcenetwork/defradb/net/api/pb"
+	pb "github.com/sourcenetwork/defradb/net/pb"
 )
 
+func init() {
+	encoding.RegisterCodec(codec.Codec{})
+}
+
 type Client struct {
-	c    pb.ServiceClient
+	c    pb.CollectionClient
 	conn *grpc.ClientConn
 }
 
@@ -38,7 +45,7 @@ func NewClient(target string, opts ...grpc.DialOption) (*Client, error) {
 	}
 
 	return &Client{
-		c:    pb.NewServiceClient(conn),
+		c:    pb.NewCollectionClient(conn),
 		conn: conn,
 	}, nil
 }

diff --git a/net/api/pb/Makefile b/net/api/pb/Makefile
@@ -4,9 +4,12 @@ GO = $(PB:.proto=.pb.go)
 all: $(GO)
 
 %.pb.go: %.proto
-	protoc -I=. -I=$(GOPATH)/src -I=$(GOPATH)/src/github.com/gogo/protobuf/protobuf --gogofaster_out=\
-	plugins=grpc:\
-	. $<
+	protoc \
+	--go_out=. --plugin protoc-gen-go="${GOBIN}/protoc-gen-go" \
+    --go-grpc_out=. --plugin protoc-gen-go-grpc="${GOBIN}/protoc-gen-go-grpc" \
+    --go-vtproto_out=. --plugin protoc-gen-go-vtproto="${GOBIN}/protoc-gen-go-vtproto" \
+    --go-vtproto_opt=features=marshal+unmarshal+size \
+	$<
 
 clean:
 	rm -f *.pb.go