From 808a1e9deb4c9b01c78884fdc468852593490b64 Mon Sep 17 00:00:00 2001 From: Jakub Sztandera Date: Wed, 3 Jul 2019 18:59:49 +0200 Subject: [PATCH 1/7] Add skeleton of a config Took way longer than it should had because I was researching exisiting options. As it turns out, nothing nice exists that would handle: - Multiple overridiable config files - Defaults provided in a struct - Output in a struct License: MIT Signed-off-by: Jakub Sztandera --- .golangci.yml | 1 + go.mod | 5 ++++ go.sum | 7 +++++ node/config/def.go | 38 ++++++++++++++++++++++++ node/config/load.go | 34 ++++++++++++++++++++++ node/config/load_test.go | 63 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 148 insertions(+) create mode 100644 node/config/def.go create mode 100644 node/config/load.go create mode 100644 node/config/load_test.go diff --git a/.golangci.yml b/.golangci.yml index 6ffacd4b267..d8c5cf8c53d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -18,6 +18,7 @@ linters: issues: exclude: - "func name will be used as test\\.Test.* by other packages, and that stutters; consider calling this" + - "Potential file inclusion via variable" exclude-use-default: false exclude-rules: diff --git a/go.mod b/go.mod index a729ed3957e..f0813cf7539 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,9 @@ module github.com/filecoin-project/go-lotus go 1.12 require ( + github.com/BurntSushi/toml v0.3.1 + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/golang/protobuf v1.3.1 // indirect github.com/ipfs/go-datastore v0.0.5 github.com/ipfs/go-ipfs-routing v0.1.0 github.com/ipfs/go-log v0.0.2-0.20190703113630-0c3cfb1eccc4 @@ -25,9 +28,11 @@ require ( github.com/libp2p/go-maddr-filter v0.0.4 github.com/multiformats/go-multiaddr v0.0.4 github.com/multiformats/go-multihash v0.0.5 + github.com/stretchr/testify v1.3.0 github.com/whyrusleeping/multiaddr-filter v0.0.0-20160516205228-e903e4adabd7 go.uber.org/dig v1.7.0 // indirect go.uber.org/fx v1.9.0 go.uber.org/goleak v0.10.0 // indirect gopkg.in/urfave/cli.v2 v2.0.0-20180128182452-d3ae77c26ac8 + gopkg.in/yaml.v2 v2.2.2 // indirect ) diff --git a/go.sum b/go.sum index 513d9e06227..795f01f842c 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,6 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= github.com/AndreasBriese/bbloom v0.0.0-20180913140656-343706a395b7/go.mod h1:bOvUY6CB00SOBii9/FifXqc0awNKxLFCL/+pkDPuyl8= +github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/Kubuxu/go-os-helper v0.0.1/go.mod h1:N8B+I7vPCT80IcP58r50u4+gEEcsZETFUpAzWW2ep1Y= github.com/aead/siphash v1.0.1/go.mod h1:Nywa3cDsYNNK3gaciGTWPwHt0wlpNV15vwmswBAUSII= @@ -22,6 +23,8 @@ github.com/coreos/go-semver v0.3.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3Ee github.com/davecgh/go-spew v0.0.0-20171005155431-ecdeabc65495/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davidlazar/go-crypto v0.0.0-20170701192655-dcfb0a7ac018 h1:6xT9KW8zLC5IlbaIF5Q7JNieBoACT7iW0YTxQHR0in0= github.com/davidlazar/go-crypto v0.0.0-20170701192655-dcfb0a7ac018/go.mod h1:rQYf4tfk5sSwFsnDg3qYaBxSjsD9S8+59vW0dKUgme4= github.com/dgraph-io/badger v1.5.5-0.20190226225317-8115aed38f8f/go.mod h1:VZxzAIRPHRVNRKRo6AXrX9BJegn6il06VMTZVJYCIjQ= @@ -40,6 +43,8 @@ github.com/golang/mock v1.2.0/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfb github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.0 h1:kbxbvI4Un1LUWKxufD+BiE6AEExYYgkQLQmLFqA1LFk= github.com/golang/protobuf v1.3.0/go.mod h1:Qd/q+1AKNOZr9uGQzbzCmRO6sUih6GTPZv6a1/R87v0= +github.com/golang/protobuf v1.3.1 h1:YF8+flBXS5eO826T4nzqPrxfhQThhXl0YzfuUPu4SBg= +github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/snappy v0.0.0-20180518054509-2e65f85255db/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= github.com/google/go-cmp v0.2.0 h1:+dTQ8DZQJz0Mb/HjFlkptS1FeQ4cWSnN941F8aEG4SQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= @@ -364,4 +369,6 @@ gopkg.in/urfave/cli.v2 v2.0.0-20180128182452-d3ae77c26ac8 h1:Ggy3mWN4l3PUFPfSG0Y gopkg.in/urfave/cli.v2 v2.0.0-20180128182452-d3ae77c26ac8/go.mod h1:cKXr3E0k4aosgycml1b5z33BVV6hai1Kh7uDgFOkbcs= gopkg.in/yaml.v2 v2.2.1 h1:mUhvW9EsL+naU5Q3cakzfE91YhliOondGd6ZrsDBHQE= gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= +gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/node/config/def.go b/node/config/def.go new file mode 100644 index 00000000000..5c1042e0afd --- /dev/null +++ b/node/config/def.go @@ -0,0 +1,38 @@ +package config + +import "time" + +// Root is starting point of the config +type Root struct { + API API +} + +// API contains configs for API endpoint +type API struct { + ListenAddress string + Timeout Duration +} + +// Default returns the default config +func Default() *Root { + def := Root{ + API: API{ + ListenAddress: "/ip6/::1/tcp/1234/http", + Timeout: Duration(30 * time.Second), + }, + } + return &def +} + +// Duration is a wrapper type for time.Duration for decoding it from TOML +type Duration time.Duration + +// UnmarshalText implements interface for TOML decoding +func (dur *Duration) UnmarshalText(text []byte) error { + d, err := time.ParseDuration(string(text)) + if err != nil { + return err + } + *dur = Duration(d) + return err +} diff --git a/node/config/load.go b/node/config/load.go new file mode 100644 index 00000000000..2f2d223b39e --- /dev/null +++ b/node/config/load.go @@ -0,0 +1,34 @@ +package config + +import ( + "io" + "os" + + "github.com/BurntSushi/toml" +) + +// FromFile loads config from a specified file overriding defaults specified in +// the source code. If file does not exist or is empty defaults are asummed. +func FromFile(path string) (*Root, error) { + file, err := os.Open(path) + switch { + case os.IsNotExist(err): + return Default(), nil + case err != nil: + return nil, err + } + + defer file.Close() //nolint:errcheck // The file is RO + return FromReader(file) +} + +// FromReader loads config from a reader instance. +func FromReader(reader io.Reader) (*Root, error) { + cfg := Default() + _, err := toml.DecodeReader(reader, cfg) + if err != nil { + return nil, err + } + + return cfg, nil +} diff --git a/node/config/load_test.go b/node/config/load_test.go new file mode 100644 index 00000000000..543a00103e9 --- /dev/null +++ b/node/config/load_test.go @@ -0,0 +1,63 @@ +package config + +import ( + "bytes" + "io/ioutil" + "os" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestDecodeNothing(t *testing.T) { + assert := assert.New(t) + + { + cfg, err := FromFile(os.DevNull) + assert.Nil(err, "error should be nil") + assert.Equal(Default(), cfg, + "config from empty file should be the same as default") + } + + { + cfg, err := FromFile("./does-not-exist.toml") + assert.Nil(err, "error should be nil") + assert.Equal(Default(), cfg, + "config from not exisiting file should be the same as default") + } +} + +func TestParitalConfig(t *testing.T) { + assert := assert.New(t) + cfgString := ` + [API] + Timeout = "10s" + ` + expected := Default() + expected.API.Timeout = Duration(10 * time.Second) + + { + cfg, err := FromReader(bytes.NewReader([]byte(cfgString))) + assert.NoError(err, "error should be nil") + assert.Equal(expected, cfg, + "config from reader should contain changes") + } + + { + f, err := ioutil.TempFile("", "config-*.toml") + fname := f.Name() + + assert.NoError(err, "tmp file shold not error") + _, err = f.WriteString(cfgString) + assert.NoError(err, "writing to tmp file should not error") + err = f.Close() + assert.NoError(err, "closing tmp file should not error") + defer os.Remove(fname) //nolint:errcheck + + cfg, err := FromFile(fname) + assert.Nil(err, "error should be nil") + assert.Equal(expected, cfg, + "config from reader should contain changes") + } +} From f08263662f9fbbd1ab0df8d9860efc42ca23d89e Mon Sep 17 00:00:00 2001 From: Jakub Sztandera Date: Thu, 4 Jul 2019 14:04:39 +0200 Subject: [PATCH 2/7] Use config for listen addresses in libp2p License: MIT Signed-off-by: Jakub Sztandera --- node/builder.go | 11 +++++------ node/config/def.go | 14 +++++++++++++- node/modules/lp2p/addrs.go | 33 ++++++++++++++++----------------- 3 files changed, 34 insertions(+), 24 deletions(-) diff --git a/node/builder.go b/node/builder.go index 5b94dcae1a2..1711e296b26 100644 --- a/node/builder.go +++ b/node/builder.go @@ -13,16 +13,12 @@ import ( "github.com/filecoin-project/go-lotus/api" "github.com/filecoin-project/go-lotus/build" + "github.com/filecoin-project/go-lotus/node/config" "github.com/filecoin-project/go-lotus/node/modules" "github.com/filecoin-project/go-lotus/node/modules/helpers" "github.com/filecoin-project/go-lotus/node/modules/lp2p" ) -var defaultListenAddrs = []string{ // TODO: better defaults? - "/ip4/0.0.0.0/tcp/4001", - "/ip6/::/tcp/4001", -} - // New builds and starts new Filecoin node func New(ctx context.Context) (api.API, error) { var resAPI api.Struct @@ -31,6 +27,9 @@ func New(ctx context.Context) (api.API, error) { app := fx.New( fx.Provide(as(ctx, new(helpers.MetricsCtx))), + fx.Provide(func() (*config.Root, error) { + return config.FromFile("./config.toml") + }), //fx.Provide(modules.RandomPeerID), randomIdentity(), @@ -63,7 +62,7 @@ func New(ctx context.Context) (api.API, error) { fx.Invoke( lp2p.PstoreAddSelfKeys, - lp2p.StartListening(defaultListenAddrs), + lp2p.StartListening, ), ), diff --git a/node/config/def.go b/node/config/def.go index 5c1042e0afd..02296b7493d 100644 --- a/node/config/def.go +++ b/node/config/def.go @@ -4,7 +4,8 @@ import "time" // Root is starting point of the config type Root struct { - API API + API API + Libp2p Libp2p } // API contains configs for API endpoint @@ -13,6 +14,11 @@ type API struct { Timeout Duration } +// Libp2p contains configs for libp2p +type Libp2p struct { + ListenAddresses []string +} + // Default returns the default config func Default() *Root { def := Root{ @@ -20,6 +26,12 @@ func Default() *Root { ListenAddress: "/ip6/::1/tcp/1234/http", Timeout: Duration(30 * time.Second), }, + Libp2p: Libp2p{ + ListenAddresses: []string{ + "/ip4/0.0.0.0/tcp/4001", + "/ip6/::/tcp/4001", + }, + }, } return &def } diff --git a/node/modules/lp2p/addrs.go b/node/modules/lp2p/addrs.go index 8d34ef94e73..efbe4a03bc4 100644 --- a/node/modules/lp2p/addrs.go +++ b/node/modules/lp2p/addrs.go @@ -3,6 +3,7 @@ package lp2p import ( "fmt" + "github.com/filecoin-project/go-lotus/node/config" "github.com/libp2p/go-libp2p" host "github.com/libp2p/go-libp2p-core/host" p2pbhost "github.com/libp2p/go-libp2p/p2p/host/basic" @@ -94,24 +95,22 @@ func listenAddresses(addresses []string) ([]ma.Multiaddr, error) { return listen, nil } -func StartListening(addresses []string) func(host host.Host) error { - return func(host host.Host) error { - listenAddrs, err := listenAddresses(addresses) - if err != nil { - return err - } +func StartListening(host host.Host, cfg *config.Root) error { + listenAddrs, err := listenAddresses(cfg.Libp2p.ListenAddresses) + if err != nil { + return err + } - // Actually start listening: - if err := host.Network().Listen(listenAddrs...); err != nil { - return err - } + // Actually start listening: + if err := host.Network().Listen(listenAddrs...); err != nil { + return err + } - // list out our addresses - addrs, err := host.Network().InterfaceListenAddresses() - if err != nil { - return err - } - log.Infof("Swarm listening at: %s", addrs) - return nil + // list out our addresses + addrs, err := host.Network().InterfaceListenAddresses() + if err != nil { + return err } + log.Infof("Swarm listening at: %s", addrs) + return nil } From de604065fb03aa8aa9578604bc2fffac8fc7eb82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Magiera?= Date: Thu, 4 Jul 2019 17:50:48 +0200 Subject: [PATCH 3/7] Rewrite constructor to functional opts --- daemon/cmd.go | 8 +- node/builder.go | 247 +++++++++++++++++++------------------ node/modules/lp2p/addrs.go | 35 +++--- node/options.go | 106 ++++++++++++++++ 4 files changed, 258 insertions(+), 138 deletions(-) create mode 100644 node/options.go diff --git a/daemon/cmd.go b/daemon/cmd.go index 4e46e39ca6d..4c003160cac 100644 --- a/daemon/cmd.go +++ b/daemon/cmd.go @@ -2,6 +2,7 @@ package daemon import ( "context" + "github.com/filecoin-project/go-lotus/node/config" "gopkg.in/urfave/cli.v2" @@ -15,7 +16,12 @@ var Cmd = &cli.Command{ Action: func(cctx *cli.Context) error { ctx := context.Background() - api, err := node.New(ctx) + cfg, err := config.FromFile("./config.toml") + if err != nil { + return err + } + + api, err := node.New(ctx, node.Online(), node.Config(cfg)) if err != nil { return err } diff --git a/node/builder.go b/node/builder.go index 1711e296b26..8c2dfe406eb 100644 --- a/node/builder.go +++ b/node/builder.go @@ -2,10 +2,15 @@ package node import ( "context" - "reflect" + "errors" + "github.com/filecoin-project/go-lotus/node/config" + "github.com/ipfs/go-datastore" + "github.com/libp2p/go-libp2p-core/host" + "github.com/libp2p/go-libp2p-core/peerstore" + "github.com/libp2p/go-libp2p-core/routing" + record "github.com/libp2p/go-libp2p-record" "time" - "github.com/ipfs/go-datastore" ci "github.com/libp2p/go-libp2p-core/crypto" "github.com/libp2p/go-libp2p-core/peer" "github.com/libp2p/go-libp2p-peerstore/pstoremem" @@ -13,58 +18,130 @@ import ( "github.com/filecoin-project/go-lotus/api" "github.com/filecoin-project/go-lotus/build" - "github.com/filecoin-project/go-lotus/node/config" "github.com/filecoin-project/go-lotus/node/modules" "github.com/filecoin-project/go-lotus/node/modules/helpers" "github.com/filecoin-project/go-lotus/node/modules/lp2p" ) +// special is a type used to give keys to modules which +// can't really be identified by the returned type +type special struct{ id int } + +var ( + DefaultTransportsKey = special{0} // Libp2p option + PNetKey = special{1} // Option + multiret + DiscoveryHandlerKey = special{2} // Private type + AddrsFactoryKey = special{3} // Libp2p option + SmuxTransportKey = special{4} // Libp2p option + RelayKey = special{5} // Libp2p option + SecurityKey = special{6} // Libp2p option + BaseRoutingKey = special{7} // fx groups + multiret + NatPortMapKey = special{8} // Libp2p option + ConnectionManagerKey = special{9} // Libp2p option + +) + +type invoke int + +const ( + PstoreAddSelfKeysKey = invoke(iota) + StartListeningKey + + _nInvokes // keep this last +) + +type settings struct { + modules map[interface{}]fx.Option + + // invokes are separate from modules as they can't be referenced by return + // type, and must be applied in correct order + invokes []fx.Option + + online bool // Online option applied + config bool // Config option applied +} + +var defConf = config.Default() + +var defaults = []Option{ + Override(new(helpers.MetricsCtx), context.Background), + + randomIdentity(), + + Override(new(datastore.Batching), datastore.NewMapDatastore), + Override(new(record.Validator), modules.RecordValidator), +} + +func Online() Option { + return Options( + func(s *settings) error { s.online = true; return nil }, + applyIf(func(s *settings) bool { return s.config }, + Error(errors.New("the Online option must be set before Config option")), + ), + + Override(new(peerstore.Peerstore), pstoremem.NewPeerstore), + + Override(DefaultTransportsKey, lp2p.DefaultTransports), + Override(PNetKey, lp2p.PNet), + + Override(new(lp2p.RawHost), lp2p.Host), + Override(new(host.Host), lp2p.RoutedHost), + Override(new(lp2p.BaseIpfsRouting), lp2p.DHTRouting(false)), + + Override(DiscoveryHandlerKey, lp2p.DiscoveryHandler), + Override(AddrsFactoryKey, lp2p.AddrsFactory(nil, nil)), + Override(SmuxTransportKey, lp2p.SmuxTransport(true)), + Override(RelayKey, lp2p.Relay(true, false)), + Override(SecurityKey, lp2p.Security(true, false)), + + Override(BaseRoutingKey, lp2p.BaseRouting), + Override(new(routing.Routing), lp2p.Routing), + + Override(NatPortMapKey, lp2p.NatPortMap), + Override(ConnectionManagerKey, lp2p.ConnectionManager(50, 200, 20*time.Second)), + + Override(PstoreAddSelfKeysKey, lp2p.PstoreAddSelfKeys), + Override(StartListeningKey, lp2p.StartListening(defConf.Libp2p.ListenAddresses)), + ) +} + +func Config(cfg *config.Root) Option { + return Options( + func(s *settings) error { s.config = true; return nil }, + + applyIf(func(s *settings) bool { return s.online }, + Override(StartListeningKey, lp2p.StartListening(cfg.Libp2p.ListenAddresses)), + ), + ) +} + // New builds and starts new Filecoin node -func New(ctx context.Context) (api.API, error) { +func New(ctx context.Context, opts ...Option) (api.API, error) { var resAPI api.Struct + settings := settings{ + modules: map[interface{}]fx.Option{}, + invokes: make([]fx.Option, _nInvokes), + } - online := true + if err := Options(Options(defaults...), Options(opts...))(&settings); err != nil { + return nil, err + } + + ctors := make([]fx.Option, 0, len(settings.modules)) + for _, opt := range settings.modules { + ctors = append(ctors, opt) + } + + // fill holes in invokes + for i, opt := range settings.invokes { + if opt == nil { + settings.invokes[i] = fx.Options() + } + } app := fx.New( - fx.Provide(as(ctx, new(helpers.MetricsCtx))), - fx.Provide(func() (*config.Root, error) { - return config.FromFile("./config.toml") - }), - - //fx.Provide(modules.RandomPeerID), - randomIdentity(), - memrepo(), - - fx.Provide(modules.RecordValidator), - - ifOpt(online, - fx.Provide( - pstoremem.NewPeerstore, - - lp2p.DefaultTransports, - lp2p.PNet, - lp2p.Host, - lp2p.RoutedHost, - lp2p.DHTRouting(false), - - lp2p.DiscoveryHandler, - lp2p.AddrsFactory(nil, nil), - lp2p.SmuxTransport(true), - lp2p.Relay(true, false), - lp2p.Security(true, false), - - lp2p.BaseRouting, - lp2p.Routing, - - lp2p.NatPortMap, - lp2p.ConnectionManager(50, 200, 20*time.Second), - ), - - fx.Invoke( - lp2p.PstoreAddSelfKeys, - lp2p.StartListening, - ), - ), + fx.Options(ctors...), + fx.Options(settings.invokes...), fx.Invoke(versionAPI(&resAPI.Internal.Version)), fx.Invoke(idAPI(&resAPI.Internal.ID)), @@ -79,36 +156,19 @@ func New(ctx context.Context) (api.API, error) { // In-memory / testing -func memrepo() fx.Option { - return fx.Provide( - func() datastore.Batching { - return datastore.NewMapDatastore() - }, - ) -} - -func randomIdentity() fx.Option { +func randomIdentity() Option { sk, pk, err := ci.GenerateKeyPair(ci.RSA, 512) if err != nil { - return fx.Error(err) + return Error(err) } - return fx.Options( - fx.Provide(as(sk, new(ci.PrivKey))), - fx.Provide(as(pk, new(ci.PubKey))), - fx.Provide(peer.IDFromPublicKey), + return Options( + Override(new(ci.PrivKey), sk), + Override(new(ci.PubKey), pk), + Override(new(peer.ID), peer.IDFromPublicKey), ) } -// UTILS - -func ifOpt(cond bool, options ...fx.Option) fx.Option { - if cond { - return fx.Options(options...) - } - return fx.Options() -} - // API IMPL // TODO: figure out a better way, this isn't usable in long term @@ -129,56 +189,3 @@ func versionAPI(set *func(context.Context) (api.Version, error)) func() { } } } - -// from go-ipfs -// as casts input constructor to a given interface (if a value is given, it -// wraps it into a constructor). -// -// Note: this method may look like a hack, and in fact it is one. -// This is here only because https://github.com/uber-go/fx/issues/673 wasn't -// released yet -// -// Note 2: when making changes here, make sure this method stays at -// 100% coverage. This makes it less likely it will be terribly broken -func as(in interface{}, as interface{}) interface{} { - outType := reflect.TypeOf(as) - - if outType.Kind() != reflect.Ptr { - panic("outType is not a pointer") - } - - if reflect.TypeOf(in).Kind() != reflect.Func { - ctype := reflect.FuncOf(nil, []reflect.Type{outType.Elem()}, false) - - return reflect.MakeFunc(ctype, func(args []reflect.Value) (results []reflect.Value) { - out := reflect.New(outType.Elem()) - out.Elem().Set(reflect.ValueOf(in)) - - return []reflect.Value{out.Elem()} - }).Interface() - } - - inType := reflect.TypeOf(in) - - ins := make([]reflect.Type, inType.NumIn()) - outs := make([]reflect.Type, inType.NumOut()) - - for i := range ins { - ins[i] = inType.In(i) - } - outs[0] = outType.Elem() - for i := range outs[1:] { - outs[i+1] = inType.Out(i + 1) - } - - ctype := reflect.FuncOf(ins, outs, false) - - return reflect.MakeFunc(ctype, func(args []reflect.Value) (results []reflect.Value) { - outs := reflect.ValueOf(in).Call(args) - out := reflect.New(outType.Elem()) - out.Elem().Set(outs[0]) - outs[0] = out.Elem() - - return outs - }).Interface() -} diff --git a/node/modules/lp2p/addrs.go b/node/modules/lp2p/addrs.go index efbe4a03bc4..95971449d84 100644 --- a/node/modules/lp2p/addrs.go +++ b/node/modules/lp2p/addrs.go @@ -3,9 +3,8 @@ package lp2p import ( "fmt" - "github.com/filecoin-project/go-lotus/node/config" "github.com/libp2p/go-libp2p" - host "github.com/libp2p/go-libp2p-core/host" + "github.com/libp2p/go-libp2p-core/host" p2pbhost "github.com/libp2p/go-libp2p/p2p/host/basic" mafilter "github.com/libp2p/go-maddr-filter" ma "github.com/multiformats/go-multiaddr" @@ -95,22 +94,24 @@ func listenAddresses(addresses []string) ([]ma.Multiaddr, error) { return listen, nil } -func StartListening(host host.Host, cfg *config.Root) error { - listenAddrs, err := listenAddresses(cfg.Libp2p.ListenAddresses) - if err != nil { - return err - } +func StartListening(addresses []string) func(host host.Host) error { + return func(host host.Host) error { + listenAddrs, err := listenAddresses(addresses) + if err != nil { + return err + } - // Actually start listening: - if err := host.Network().Listen(listenAddrs...); err != nil { - return err - } + // Actually start listening: + if err := host.Network().Listen(listenAddrs...); err != nil { + return err + } - // list out our addresses - addrs, err := host.Network().InterfaceListenAddresses() - if err != nil { - return err + // list out our addresses + addrs, err := host.Network().InterfaceListenAddresses() + if err != nil { + return err + } + log.Infof("Swarm listening at: %s", addrs) + return nil } - log.Infof("Swarm listening at: %s", addrs) - return nil } diff --git a/node/options.go b/node/options.go new file mode 100644 index 00000000000..6260aaadfe6 --- /dev/null +++ b/node/options.go @@ -0,0 +1,106 @@ +package node + +import ( + "go.uber.org/fx" + "reflect" +) + +type Option func(*settings) error + +func Override(typ, constructor interface{}) Option { + return func(s *settings) error { + if i, ok := typ.(invoke); ok { + s.invokes[i] = fx.Invoke(constructor) + return nil + } + + if c, ok := typ.(special); ok { + s.modules[c] = fx.Provide(constructor) + return nil + } + ctor := as(constructor, typ) + rt := reflect.TypeOf(typ).Elem() + + s.modules[rt] = fx.Provide(ctor) + return nil + } +} + +func Options(opts ...Option) Option { + return func(s *settings) error { + for _, opt := range opts { + if err := opt(s); err != nil { + return err + } + } + return nil + } +} + +func Error(err error) Option { + return func(_ *settings) error { + return err + } +} + +func applyIf(check func(s *settings) bool, opts ...Option) Option { + return func(s *settings) error { + if check(s) { + return Options(opts...)(s) + } + return nil + } +} + +// from go-ipfs +// as casts input constructor to a given interface (if a value is given, it +// wraps it into a constructor). +// +// Note: this method may look like a hack, and in fact it is one. +// This is here only because https://github.com/uber-go/fx/issues/673 wasn't +// released yet +// +// Note 2: when making changes here, make sure this method stays at +// 100% coverage. This makes it less likely it will be terribly broken +func as(in interface{}, as interface{}) interface{} { + outType := reflect.TypeOf(as) + + if outType.Kind() != reflect.Ptr { + panic("outType is not a pointer") + } + + if reflect.TypeOf(in).Kind() != reflect.Func { + ctype := reflect.FuncOf(nil, []reflect.Type{outType.Elem()}, false) + + return reflect.MakeFunc(ctype, func(args []reflect.Value) (results []reflect.Value) { + out := reflect.New(outType.Elem()) + out.Elem().Set(reflect.ValueOf(in)) + + return []reflect.Value{out.Elem()} + }).Interface() + } + + inType := reflect.TypeOf(in) + + ins := make([]reflect.Type, inType.NumIn()) + outs := make([]reflect.Type, inType.NumOut()) + + for i := range ins { + ins[i] = inType.In(i) + } + outs[0] = outType.Elem() + for i := range outs[1:] { + outs[i+1] = inType.Out(i + 1) + } + + ctype := reflect.FuncOf(ins, outs, false) + + return reflect.MakeFunc(ctype, func(args []reflect.Value) (results []reflect.Value) { + outs := reflect.ValueOf(in).Call(args) + out := reflect.New(outType.Elem()) + out.Elem().Set(outs[0]) + outs[0] = out.Elem() + + return outs + }).Interface() +} From e78c378021c7bb4f9536f97ee63200c4f568e7ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Magiera?= Date: Thu, 4 Jul 2019 22:06:02 +0200 Subject: [PATCH 4/7] constructor: Add doc strings --- node/builder.go | 42 ++++++++++++++++++++++++++++++++++++++++-- node/options.go | 26 ++++++-------------------- 2 files changed, 46 insertions(+), 22 deletions(-) diff --git a/node/builder.go b/node/builder.go index 8c2dfe406eb..3055002006c 100644 --- a/node/builder.go +++ b/node/builder.go @@ -9,6 +9,7 @@ import ( "github.com/libp2p/go-libp2p-core/peerstore" "github.com/libp2p/go-libp2p-core/routing" record "github.com/libp2p/go-libp2p-record" + "reflect" "time" ci "github.com/libp2p/go-libp2p-core/crypto" @@ -27,6 +28,7 @@ import ( // can't really be identified by the returned type type special struct{ id int } +// nolint var ( DefaultTransportsKey = special{0} // Libp2p option PNetKey = special{1} // Option + multiret @@ -38,19 +40,26 @@ var ( BaseRoutingKey = special{7} // fx groups + multiret NatPortMapKey = special{8} // Libp2p option ConnectionManagerKey = special{9} // Libp2p option - ) type invoke int const ( + // PstoreAddSelfKeysKey is a key for Override for PstoreAddSelfKeys PstoreAddSelfKeysKey = invoke(iota) + + // StartListeningKey is a key for Override for StartListening StartListeningKey _nInvokes // keep this last ) type settings struct { + // modules is a map of constructors for DI + // + // In most cases the index will be a reflect. Type of element returned by + // the constructor, but for some 'constructors' it's hard to specify what's + // the return type should be (or the constructor returns fx group) modules map[interface{}]fx.Option // invokes are separate from modules as they can't be referenced by return @@ -61,6 +70,26 @@ type settings struct { config bool // Config option applied } +// Override option changes constructor for a given type +func Override(typ, constructor interface{}) Option { + return func(s *settings) error { + if i, ok := typ.(invoke); ok { + s.invokes[i] = fx.Invoke(constructor) + return nil + } + + if c, ok := typ.(special); ok { + s.modules[c] = fx.Provide(constructor) + return nil + } + ctor := as(constructor, typ) + rt := reflect.TypeOf(typ).Elem() + + s.modules[rt] = fx.Provide(ctor) + return nil + } +} + var defConf = config.Default() var defaults = []Option{ @@ -72,8 +101,11 @@ var defaults = []Option{ Override(new(record.Validator), modules.RecordValidator), } +// Online sets up basic libp2p node func Online() Option { return Options( + // make sure that online is applied before Config. + // This is important because Config overrides some of Online units func(s *settings) error { s.online = true; return nil }, applyIf(func(s *settings) bool { return s.config }, Error(errors.New("the Online option must be set before Config option")), @@ -105,6 +137,7 @@ func Online() Option { ) } +// Config sets up constructors based on the provided config func Config(cfg *config.Root) Option { return Options( func(s *settings) error { s.config = true; return nil }, @@ -123,16 +156,18 @@ func New(ctx context.Context, opts ...Option) (api.API, error) { invokes: make([]fx.Option, _nInvokes), } + // apply module options in the right order if err := Options(Options(defaults...), Options(opts...))(&settings); err != nil { return nil, err } + // gather constructors for fx.Options ctors := make([]fx.Option, 0, len(settings.modules)) for _, opt := range settings.modules { ctors = append(ctors, opt) } - // fill holes in invokes + // fill holes in invokes for use in fx.Options for i, opt := range settings.invokes { if opt == nil { settings.invokes[i] = fx.Options() @@ -147,6 +182,9 @@ func New(ctx context.Context, opts ...Option) (api.API, error) { fx.Invoke(idAPI(&resAPI.Internal.ID)), ) + // TODO: we probably should have a 'firewall' for Closing signal + // on this context, and implement closing logic through lifecycles + // correctly if err := app.Start(ctx); err != nil { return nil, err } diff --git a/node/options.go b/node/options.go index 6260aaadfe6..b1436c96bda 100644 --- a/node/options.go +++ b/node/options.go @@ -1,31 +1,16 @@ package node import ( - "go.uber.org/fx" "reflect" ) +// Option is a functional option which can be used with the New function to +// change how the node is constructed +// +// Options are applied in sequence type Option func(*settings) error -func Override(typ, constructor interface{}) Option { - return func(s *settings) error { - if i, ok := typ.(invoke); ok { - s.invokes[i] = fx.Invoke(constructor) - return nil - } - - if c, ok := typ.(special); ok { - s.modules[c] = fx.Provide(constructor) - return nil - } - ctor := as(constructor, typ) - rt := reflect.TypeOf(typ).Elem() - - s.modules[rt] = fx.Provide(ctor) - return nil - } -} - +// Options groups multiple options into one func Options(opts ...Option) Option { return func(s *settings) error { for _, opt := range opts { @@ -37,6 +22,7 @@ func Options(opts ...Option) Option { } } +// Error is a special option which returns an error when applied func Error(err error) Option { return func(_ *settings) error { return err From fd7daf4a310abd9d7082757e8e97aed726bd627b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Magiera?= Date: Fri, 5 Jul 2019 12:06:28 +0200 Subject: [PATCH 5/7] Fix import ordering --- daemon/cmd.go | 4 ++-- node/builder.go | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/daemon/cmd.go b/daemon/cmd.go index 4c003160cac..6e8ac227087 100644 --- a/daemon/cmd.go +++ b/daemon/cmd.go @@ -2,11 +2,11 @@ package daemon import ( "context" + + "github.com/filecoin-project/go-lotus/node" "github.com/filecoin-project/go-lotus/node/config" "gopkg.in/urfave/cli.v2" - - "github.com/filecoin-project/go-lotus/node" ) // Cmd is the `go-lotus daemon` command diff --git a/node/builder.go b/node/builder.go index 3055002006c..3ca0b7da839 100644 --- a/node/builder.go +++ b/node/builder.go @@ -3,22 +3,22 @@ package node import ( "context" "errors" - "github.com/filecoin-project/go-lotus/node/config" - "github.com/ipfs/go-datastore" - "github.com/libp2p/go-libp2p-core/host" - "github.com/libp2p/go-libp2p-core/peerstore" - "github.com/libp2p/go-libp2p-core/routing" - record "github.com/libp2p/go-libp2p-record" "reflect" "time" + "github.com/ipfs/go-datastore" ci "github.com/libp2p/go-libp2p-core/crypto" + "github.com/libp2p/go-libp2p-core/host" "github.com/libp2p/go-libp2p-core/peer" + "github.com/libp2p/go-libp2p-core/peerstore" + "github.com/libp2p/go-libp2p-core/routing" "github.com/libp2p/go-libp2p-peerstore/pstoremem" + record "github.com/libp2p/go-libp2p-record" "go.uber.org/fx" "github.com/filecoin-project/go-lotus/api" "github.com/filecoin-project/go-lotus/build" + "github.com/filecoin-project/go-lotus/node/config" "github.com/filecoin-project/go-lotus/node/modules" "github.com/filecoin-project/go-lotus/node/modules/helpers" "github.com/filecoin-project/go-lotus/node/modules/lp2p" From 71dfa38032d112c4d066e7949590396c37c2a41a Mon Sep 17 00:00:00 2001 From: Jakub Sztandera Date: Fri, 5 Jul 2019 13:21:54 +0200 Subject: [PATCH 6/7] Update linter License: MIT Signed-off-by: Jakub Sztandera --- .circleci/config.yml | 1 + .golangci.yml | 3 ++- node/builder.go | 2 +- node/modules/lp2p/libp2p.go | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index d2d6e997bbe..5752a6dbbae 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -17,6 +17,7 @@ workflows: ci: jobs: - go/lint: + golangci-lint-version: 1.17.1 executor: go/circleci-golang - go/test: executor: go/circleci-golang diff --git a/.golangci.yml b/.golangci.yml index d8c5cf8c53d..dcc21617942 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -2,7 +2,7 @@ linters: disable-all: true enable: - vet - - gofmt + - goimports - misspell - goconst - golint @@ -13,6 +13,7 @@ linters: - varcheck - structcheck - deadcode + - scopelint issues: diff --git a/node/builder.go b/node/builder.go index 3ca0b7da839..7a2cc4f8a62 100644 --- a/node/builder.go +++ b/node/builder.go @@ -28,7 +28,7 @@ import ( // can't really be identified by the returned type type special struct{ id int } -// nolint +//nolint:golint var ( DefaultTransportsKey = special{0} // Libp2p option PNetKey = special{1} // Option + multiret diff --git a/node/modules/lp2p/libp2p.go b/node/modules/lp2p/libp2p.go index 229f05afe22..eb6aee27f2f 100644 --- a/node/modules/lp2p/libp2p.go +++ b/node/modules/lp2p/libp2p.go @@ -5,7 +5,7 @@ import ( logging "github.com/ipfs/go-log" "github.com/libp2p/go-libp2p" - "github.com/libp2p/go-libp2p-connmgr" + connmgr "github.com/libp2p/go-libp2p-connmgr" "github.com/libp2p/go-libp2p-core/crypto" "github.com/libp2p/go-libp2p-core/peer" "github.com/libp2p/go-libp2p-core/peerstore" From 75f9a4ec821b9aa8f56e47297345b5c8522fd439 Mon Sep 17 00:00:00 2001 From: Jakub Sztandera Date: Fri, 5 Jul 2019 13:31:38 +0200 Subject: [PATCH 7/7] Change executor License: MIT Signed-off-by: Jakub Sztandera --- .circleci/config.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 5752a6dbbae..e1ec7e57fb7 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -18,7 +18,6 @@ workflows: jobs: - go/lint: golangci-lint-version: 1.17.1 - executor: go/circleci-golang - go/test: executor: go/circleci-golang codecov-upload: true