Skip to content

Commit

Permalink
remove reflection
Browse files Browse the repository at this point in the history
  • Loading branch information
halseth committed Oct 31, 2018
1 parent fc10625 commit b9c74ff
Show file tree
Hide file tree
Showing 10 changed files with 55 additions and 185 deletions.
6 changes: 2 additions & 4 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ type config struct {

Tor *torConfig `group:"Tor" namespace:"tor"`

SubRpcServers *subRpcServerConfigs `group:"subrpc"`
SignRPC *signrpc.Config `group:"subrpc"`

Hodl *hodl.Config `group:"hodl" namespace:"hodl"`

Expand Down Expand Up @@ -296,9 +296,7 @@ func loadConfig() (*config, error) {
},
MaxPendingChannels: defaultMaxPendingChannels,
NoSeedBackup: defaultNoSeedBackup,
SubRpcServers: &subRpcServerConfigs{
SignRPC: &signrpc.Config{},
},
SignRPC: &signrpc.Config{},
Autopilot: &autoPilotConfig{
MaxChannels: 5,
Allocation: 0.6,
Expand Down
2 changes: 1 addition & 1 deletion lnd.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ func lndMain() error {
// Initialize, and register our implementation of the gRPC interface
// exported by the rpcServer.
rpcServer, err := newRPCServer(
server, macaroonService, cfg.SubRpcServers, serverOpts,
server, macaroonService, serverOpts,
proxyOpts, tlsConf,
)
if err != nil {
Expand Down
9 changes: 9 additions & 0 deletions lnrpc/signrpc/config_active.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,12 @@ type Config struct {
// the active signer instance.
Signer lnwallet.Signer
}

func (c *Config) Populate(sign lnwallet.Signer,
networkDir string, macService *macaroons.Service) *Config {

c.NetworkDir = networkDir
c.MacService = macService
c.Signer = sign
return c
}
11 changes: 11 additions & 0 deletions lnrpc/signrpc/config_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,16 @@

package signrpc

import (
"github.com/lightningnetwork/lnd/lnwallet"
"github.com/lightningnetwork/lnd/macaroons"
)

// Config is empty for non-signrpc builds.
type Config struct{}

func (c *Config) Populate(sign lnwallet.Signer,
networkDir string, macService *macaroons.Service) *Config {
// Intentionally left empty.
return c
}
25 changes: 7 additions & 18 deletions lnrpc/signrpc/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,13 @@ import (
// server given the main config dispatcher method. If we're unable to find the
// config that is meant for us in the config dispatcher, then we'll exit with
// an error.
func createNewSubServer(configRegistry lnrpc.SubServerConfigDispatcher) (lnrpc.SubServer, lnrpc.MacaroonPerms, error) {
// We'll attempt to look up the config that we expect, according to our
// subServerName name. If we can't find this, then we'll exit with an
// error, as we're unable to properly initialize ourselves without this
// config.
signServerConf, ok := configRegistry.FetchConfig(subServerName)
if !ok {
return nil, nil, fmt.Errorf("unable to find config for "+
"subserver type %s", subServerName)
}
func createNewSubServer(c interface{}) (lnrpc.SubServer, lnrpc.MacaroonPerms, error) {

// Now that we've found an object mapping to our service name, we'll
// ensure that it's the type we need.
config, ok := signServerConf.(*Config)
config, ok := c.(*Config)
if !ok {
return nil, nil, fmt.Errorf("wrong type of config for "+
"subserver %s, expected %T got %T", subServerName,
&Config{}, signServerConf)
"subserver %s, expected %T got %T", SubServerName,
&Config{}, c)
}

// Before we try to make the new signer service instance, we'll perform
Expand All @@ -51,8 +40,8 @@ func createNewSubServer(configRegistry lnrpc.SubServerConfigDispatcher) (lnrpc.S

func init() {
subServer := &lnrpc.SubServerDriver{
SubServerName: subServerName,
New: func(c lnrpc.SubServerConfigDispatcher) (lnrpc.SubServer, lnrpc.MacaroonPerms, error) {
SubServerName: SubServerName,
New: func(c interface{}) (lnrpc.SubServer, lnrpc.MacaroonPerms, error) {
return createNewSubServer(c)
},
}
Expand All @@ -61,6 +50,6 @@ func init() {
// sub-RPC server within the global lnrpc package namespace.
if err := lnrpc.RegisterSubServer(subServer); err != nil {
panic(fmt.Sprintf("failed to register sub server driver '%s': %v",
subServerName, err))
SubServerName, err))
}
}
9 changes: 8 additions & 1 deletion lnrpc/signrpc/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,21 @@ import (
"github.com/lightningnetwork/lnd/build"
)

const (
// subServerName is the name of the sub rpc server. We'll use this name
// to register ourselves, and we also require that the main
// SubServerConfigDispatcher instance recognize as the name of our
SubServerName = "SGNR"
)

// log is a logger that is initialized with no output filters. This
// means the package will not perform any logging by default until the caller
// requests it.
var log btclog.Logger

// The default amount of logging is none.
func init() {
UseLogger(build.NewSubLogger("SGNR", nil))
UseLogger(build.NewSubLogger(SubServerName, nil))
}

// DisableLog disables all library log output. Logging output is disabled
Expand Down
9 changes: 1 addition & 8 deletions lnrpc/signrpc/signer_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,6 @@ import (
"gopkg.in/macaroon-bakery.v2/bakery"
)

const (
// subServerName is the name of the sub rpc server. We'll use this name
// to register ourselves, and we also require that the main
// SubServerConfigDispatcher instance recognize as the name of our
subServerName = "SignRPC"
)

var (
// macaroonOps are the set of capabilities that our minted macaroon (if
// it doesn't already exist) will have.
Expand Down Expand Up @@ -158,7 +151,7 @@ func (s *Server) Stop() error {
//
// NOTE: This is part of the lnrpc.SubServer interface.
func (s *Server) Name() string {
return subServerName
return SubServerName
}

// RegisterWithRootServer will be called by the root gRPC server to direct a
Expand Down
14 changes: 1 addition & 13 deletions lnrpc/sub_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,6 @@ type SubServer interface {
RegisterWithRootServer(*grpc.Server) error
}

// SubServerConfigDispatcher is an interface that all sub-servers will use to
// dynamically locate their configuration files. This abstraction will allow
// the primary RPC sever to initialize all sub-servers in a generic manner
// without knowing of each individual sub server.
type SubServerConfigDispatcher interface {
// FetchConfig attempts to locate an existing configuration file mapped
// to the target sub-server. If we're unable to find a config file
// matching the subServerName name, then false will be returned for the
// second parameter.
FetchConfig(subServerName string) (interface{}, bool)
}

// SubServerDriver is a template struct that allows the root server to create a
// sub-server with minimal knowledge. The root server only need a fully
// populated SubServerConfigDispatcher and with the aide of the
Expand All @@ -66,7 +54,7 @@ type SubServerDriver struct {
// return the SubServer, ready for action, along with the set of
// macaroon permissions that the sub-server wishes to pass on to the
// root server for all methods routed towards it.
New func(subCfgs SubServerConfigDispatcher) (SubServer, MacaroonPerms, error)
New func(subCfgs interface{}) (SubServer, MacaroonPerms, error)
}

var (
Expand Down
27 changes: 15 additions & 12 deletions rpcserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/lightningnetwork/lnd/htlcswitch"
"github.com/lightningnetwork/lnd/lncfg"
"github.com/lightningnetwork/lnd/lnrpc"
"github.com/lightningnetwork/lnd/lnrpc/signrpc"
"github.com/lightningnetwork/lnd/lnwallet"
"github.com/lightningnetwork/lnd/lnwire"
"github.com/lightningnetwork/lnd/macaroons"
Expand Down Expand Up @@ -383,30 +384,32 @@ var _ lnrpc.LightningServer = (*rpcServer)(nil)
// base level options passed to the grPC server. This typically includes things
// like requiring TLS, etc.
func newRPCServer(s *server, macService *macaroons.Service,
subServerCgs *subRpcServerConfigs, serverOpts []grpc.ServerOption,
serverOpts []grpc.ServerOption,
restServerOpts []grpc.DialOption,
tlsCfg *tls.Config) (*rpcServer, error) {

var (
subServers []lnrpc.SubServer
subServerPerms []lnrpc.MacaroonPerms
subServerCfgs = map[string]func() interface{}{
signrpc.SubServerName: func() interface{} {
return cfg.SignRPC.Populate(

This comment has been minimized.

Copy link
@Roasbeef

Roasbeef Nov 2, 2018

Member

With this, the RPC server now needs to have knowledge of all the sub-severs, rather than having it be fully generalized as it is now.

This comment has been minimized.

Copy link
@halseth

halseth Nov 2, 2018

Author Contributor

You can move this map to another file (e.g. subrpcserver_config.go, which already had to be aware) if you don't want it to be a part of the rpcserver.

This comment has been minimized.

Copy link
@Roasbeef

Roasbeef Nov 2, 2018

Member

This removes the ability to pass configs to the sub-servers over the command line and/or config file.

This comment has been minimized.

Copy link
@halseth

halseth Nov 5, 2018

Author Contributor

Hm, why? This change shouldn't impact that in any way I think.

This comment has been minimized.

Copy link
@Roasbeef

Roasbeef Nov 5, 2018

Member

It does away with the command line and config parsing?

This comment has been minimized.

Copy link
@Roasbeef

Roasbeef Nov 5, 2018

Member

Goal of the reflection is to minimize the amount of sub-server specific code overall. As is, it's mostly (within lnd), when specifying the new params to add into the sub-server config.

This comment has been minimized.

Copy link
@halseth

halseth Nov 5, 2018

Author Contributor

No, command line and config parsing is the same, the only difference is that the population of the sub-server fields is moved from the file using reflection (which must use reflection because it cannot know which fields are present) to the subserver's config file (which doesn't need reflection since it is behind the build tag).

s.cc.signer, networkDir, macService,
)
},
}
)

// Before we create any of the sub-servers, we need to ensure that all
// the dependencies they need are properly populated within each sub
// server configuration struct.
err := subServerCgs.PopulateDependancies(
s.cc, networkDir, macService,
)
if err != nil {
return nil, err
}

// Now that the sub-servers have all their dependencies in place, we
// can create each sub-server!
registeredSubServers := lnrpc.RegisteredSubServers()
for _, subServer := range registeredSubServers {
subServerInstance, macPerms, err := subServer.New(subServerCgs)
fetchCfg, ok := subServerCfgs[subServer.SubServerName]
if !ok {
return nil, fmt.Errorf("config not found")
}
cfg := fetchCfg()
subServerInstance, macPerms, err := subServer.New(cfg)
if err != nil {
return nil, err
}
Expand Down
128 changes: 0 additions & 128 deletions subrpcserver_config.go

This file was deleted.

0 comments on commit b9c74ff

Please sign in to comment.