From bb29e367d5b7cba0850ead6b556b38c816963540 Mon Sep 17 00:00:00 2001 From: Preston Van Loon Date: Sat, 28 Jul 2018 21:18:56 -0400 Subject: [PATCH] Add Golint (#344) --- .gometalinter.json | 5 ++- .travis.yml | 2 +- client/internal/BUILD.bazel | 1 + client/internal/client_helper.go | 18 +++++++++++ client/main.go | 2 +- client/mainchain/interfaces.go | 1 + client/mainchain/smc_client.go | 3 ++ client/params/config.go | 8 ++++- shared/cmd/customflags.go | 41 +++++++++++++----------- shared/cmd/flags.go | 4 +-- shared/debug/debug.go | 13 ++++++-- shared/legacyutil/convert_transaction.go | 2 +- shared/marshal.go | 2 +- shared/p2p/service_test.go | 6 ++-- 14 files changed, 75 insertions(+), 33 deletions(-) diff --git a/.gometalinter.json b/.gometalinter.json index abf424bbb..a7fc571fa 100644 --- a/.gometalinter.json +++ b/.gometalinter.json @@ -7,6 +7,9 @@ "nakedret", "unparam", "megacheck", - "gosec" + "gosec", + "varcheck", + "structcheck", + "golint" ] } diff --git a/.travis.yml b/.travis.yml index 4cd4f468f..7d08a69fe 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,7 +9,7 @@ matrix: - lint script: - - go get github.com/alecthomas/gometalinter && gometalinter --install && gometalinter ./... --deadline=10m + go get github.com/alecthomas/gometalinter && gometalinter --install && gometalinter ./... --deadline=10m --exclude=client/internal/client_helper.go - os: linux env: V=0.15.0 before_install: diff --git a/client/internal/BUILD.bazel b/client/internal/BUILD.bazel index 1e8a742d4..d70bdeb75 100644 --- a/client/internal/BUILD.bazel +++ b/client/internal/BUILD.bazel @@ -2,6 +2,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( name = "go_default_library", + testonly = True, srcs = ["client_helper.go"], importpath = "github.com/prysmaticlabs/prysm/client/internal", visibility = ["//client:__subpackages__"], diff --git a/client/internal/client_helper.go b/client/internal/client_helper.go index 7f89eaaee..3d04ec6b7 100644 --- a/client/internal/client_helper.go +++ b/client/internal/client_helper.go @@ -33,81 +33,99 @@ type MockClient struct { BlockNumber int64 } +// Account returns a mock account. func (m *MockClient) Account() *accounts.Account { return &accounts.Account{Address: addr} } +// SMCCaller returns a mock SMCCaller. func (m *MockClient) SMCCaller() *contracts.SMCCaller { return &m.SMC.SMCCaller } +// ChainReader returns a mock chain reader. func (m *MockClient) ChainReader() ethereum.ChainReader { return nil } +// SMCTransactor returns a mock SMCTransactor. func (m *MockClient) SMCTransactor() *contracts.SMCTransactor { return &m.SMC.SMCTransactor } +// SMCFilterer returns a mock SMCFilterer. func (m *MockClient) SMCFilterer() *contracts.SMCFilterer { return &m.SMC.SMCFilterer } +// WaitForTransaction waits for a transaction. func (m *MockClient) WaitForTransaction(ctx context.Context, hash common.Hash, durationInSeconds time.Duration) error { m.CommitWithBlock() m.FastForward(1) return nil } +// TransactionReceipt returns the transaction receipt from the mock backend. func (m *MockClient) TransactionReceipt(hash common.Hash) (*gethTypes.Receipt, error) { return m.Backend.TransactionReceipt(context.Background(), hash) } +// CreateTXOpts returns transaction opts with the value assigned. func (m *MockClient) CreateTXOpts(value *big.Int) (*bind.TransactOpts, error) { txOpts := TransactOpts() txOpts.Value = value return txOpts, nil } +// SetDepositFlag sets the deposit flag to the boolean value. func (m *MockClient) SetDepositFlag(value bool) { m.depositFlag = value } +// DepositFlag value. func (m *MockClient) DepositFlag() bool { return m.depositFlag } +// Sign does nothing? func (m *MockClient) Sign(hash common.Hash) ([]byte, error) { return nil, nil } +// GetShardCount returns constant shard count. func (m *MockClient) GetShardCount() (int64, error) { return 100, nil } +// CommitWithBlock commits a block to the backend. func (m *MockClient) CommitWithBlock() { m.Backend.Commit() m.BlockNumber = m.BlockNumber + 1 } +// FastForward by iterating the mock backend p times. func (m *MockClient) FastForward(p int) { for i := 0; i < p*int(shardparams.DefaultPeriodLength); i++ { m.CommitWithBlock() } } +// SubscribeNewHead does nothing. func (m *MockClient) SubscribeNewHead(ctx context.Context, ch chan<- *gethTypes.Header) (ethereum.Subscription, error) { return nil, nil } +// BlockByNumber creates a block with a given number. func (m *MockClient) BlockByNumber(ctx context.Context, number *big.Int) (*gethTypes.Block, error) { return gethTypes.NewBlockWithHeader(&gethTypes.Header{Number: big.NewInt(m.BlockNumber)}), nil } +// TransactOpts Creates a new transaction options. func TransactOpts() *bind.TransactOpts { return bind.NewKeyedTransactor(key) } +// SetupMockClient sets up the mock client. func SetupMockClient(t *testing.T) (*backends.SimulatedBackend, *contracts.SMC) { backend := backends.NewSimulatedBackend(core.GenesisAlloc{addr: {Balance: accountBalance}}) _, _, SMC, err := contracts.DeploySMC(TransactOpts(), backend) diff --git a/client/main.go b/client/main.go index f7ed74a9c..8a5274788 100644 --- a/client/main.go +++ b/client/main.go @@ -54,7 +54,7 @@ VERSION: app.Usage = `launches a sharding client that interacts with a beacon chain, starts proposer services, shardp2p connections, and more ` app.Action = startNode - app.Flags = []cli.Flag{utils.ActorFlag, cmd.VerbosityFlag, cmd.DataDirFlag, cmd.PasswordFileFlag, cmd.NetworkIdFlag, cmd.IPCPathFlag, cmd.RPCProviderFlag, utils.DepositFlag, utils.ShardIDFlag, debug.PProfFlag, debug.PProfAddrFlag, debug.PProfPortFlag, debug.MemProfileRateFlag, debug.CPUProfileFlag, debug.TraceFlag} + app.Flags = []cli.Flag{utils.ActorFlag, cmd.VerbosityFlag, cmd.DataDirFlag, cmd.PasswordFileFlag, cmd.NetworkIDFlag, cmd.IPCPathFlag, cmd.RPCProviderFlag, utils.DepositFlag, utils.ShardIDFlag, debug.PProfFlag, debug.PProfAddrFlag, debug.PProfPortFlag, debug.MemProfileRateFlag, debug.CPUProfileFlag, debug.TraceFlag} app.Before = func(ctx *cli.Context) error { runtime.GOMAXPROCS(runtime.NumCPU()) diff --git a/client/mainchain/interfaces.go b/client/mainchain/interfaces.go index 5ee697d9b..30299caca 100644 --- a/client/mainchain/interfaces.go +++ b/client/mainchain/interfaces.go @@ -49,6 +49,7 @@ type EthClient interface { DepositFlag() bool } +// FullClient defines the method that will be used for full client usage. type FullClient interface { EthClient Reader diff --git a/client/mainchain/smc_client.go b/client/mainchain/smc_client.go index 80749d2b4..59d23f848 100644 --- a/client/mainchain/smc_client.go +++ b/client/mainchain/smc_client.go @@ -136,10 +136,13 @@ func (s *SMCClient) ChainReader() ethereum.ChainReader { return ethereum.ChainReader(s.client) } +// BlockByNumber helper function for fetching a mainchain block by its block +// number. func (s *SMCClient) BlockByNumber(ctx context.Context, number *big.Int) (*gethTypes.Block, error) { return s.ChainReader().BlockByNumber(ctx, number) } +// SubscribeNewHead helper function for subscribing to new mainchain headers. func (s *SMCClient) SubscribeNewHead(ctx context.Context, ch chan<- *gethTypes.Header) (ethereum.Subscription, error) { return s.ChainReader().SubscribeNewHead(ctx, ch) } diff --git a/client/params/config.go b/client/params/config.go index 425c5c3d1..33608f722 100644 --- a/client/params/config.go +++ b/client/params/config.go @@ -11,7 +11,10 @@ import ( ) const ( - DefaultPeriodLength = 5 + // DefaultPeriodLength is the default value for period lengths in sharding. + DefaultPeriodLength = 5 + // DefaultAttesterLockupLength is the default number of blocks to lock up + // an attesters deposit before they can withdraw it. DefaultAttesterLockupLength = 16128 ) @@ -30,10 +33,13 @@ func DefaultConfig() *Config { } } +// DefaultAttesterDeposit required to be an attester. func DefaultAttesterDeposit() *big.Int { return new(big.Int).Exp(big.NewInt(10), big.NewInt(21), nil) // 1000 ETH } +// DefaultCollationSizeLimit is the integer value representing the maximum +// number of bytes allowed in a given collation. func DefaultCollationSizeLimit() int64 { return int64(math.Pow(float64(2), float64(20))) } diff --git a/shared/cmd/customflags.go b/shared/cmd/customflags.go index b3429c02b..fd19ad548 100644 --- a/shared/cmd/customflags.go +++ b/shared/cmd/customflags.go @@ -9,19 +9,20 @@ import ( "strings" ) -// Custom type which is registered in the flags library which cli uses for -// argument parsing. This allows us to expand Value to an absolute path when -// the argument is parsed +// DirectoryString -- Custom type which is registered in the flags library +// which cli uses for argument parsing. This allows us to expand Value to +// an absolute path when the argument is parsed. type DirectoryString struct { Value string } -func (self *DirectoryString) String() string { - return self.Value +func (d *DirectoryString) String() string { + return d.Value } -func (self *DirectoryString) Set(value string) error { - self.Value = expandPath(value) +// Set directory string value +func (d *DirectoryString) Set(value string) error { + d.Value = expandPath(value) return nil } @@ -46,7 +47,7 @@ func prefixedNames(fullName string) (prefixed string) { return prefixed } -// Custom cli.Flag type which expand the received string to an absolute path. +// DirectoryFlag expands the received string to an absolute path. // e.g. ~/.ethereum -> /home/username/.ethereum type DirectoryFlag struct { Name string @@ -54,12 +55,12 @@ type DirectoryFlag struct { Usage string } -func (self DirectoryFlag) String() string { +func (d DirectoryFlag) String() string { fmtString := "%s %v\t%v" - if len(self.Value.Value) > 0 { + if len(d.Value.Value) > 0 { fmtString = "%s \"%v\"\t%v" } - return fmt.Sprintf(fmtString, prefixedNames(self.Name), self.Value.Value, self.Usage) + return fmt.Sprintf(fmtString, prefixedNames(d.Name), d.Value.Value, d.Usage) } func eachName(longName string, fn func(string)) { @@ -70,20 +71,22 @@ func eachName(longName string, fn func(string)) { } } -// called by cli library, grabs variable from environment (if in env) +// Apply is called by cli library, grabs variable from environment (if in env) // and adds variable to flag set for parsing. -func (self DirectoryFlag) Apply(set *flag.FlagSet) { - eachName(self.Name, func(name string) { - set.Var(&self.Value, self.Name, self.Usage) +func (d DirectoryFlag) Apply(set *flag.FlagSet) { + eachName(d.Name, func(name string) { + set.Var(&d.Value, d.Name, d.Usage) }) } -func (self DirectoryFlag) GetName() string { - return self.Name +// GetName of directory. +func (d DirectoryFlag) GetName() string { + return d.Name } -func (self *DirectoryFlag) Set(value string) { - self.Value.Value = value +// Set flag value. +func (d *DirectoryFlag) Set(value string) { + d.Value.Value = value } // Expands a file path diff --git a/shared/cmd/flags.go b/shared/cmd/flags.go index 987cb6719..d4a155bda 100644 --- a/shared/cmd/flags.go +++ b/shared/cmd/flags.go @@ -23,8 +23,8 @@ var ( Usage: "Data directory for the databases and keystore", Value: DirectoryString{node.DefaultDataDir()}, } - // NetworkIdFlag defines the specific network identifier. - NetworkIdFlag = cli.Uint64Flag{ + // NetworkIDFlag defines the specific network identifier. + NetworkIDFlag = cli.Uint64Flag{ Name: "networkid", Usage: "Network identifier (integer, 1=Frontier, 2=Morden (disused), 3=Ropsten, 4=Rinkeby)", Value: 1, diff --git a/shared/debug/debug.go b/shared/debug/debug.go index 3e75b76b3..0ed6995d7 100644 --- a/shared/debug/debug.go +++ b/shared/debug/debug.go @@ -40,32 +40,39 @@ import ( // Handler is the global debugging handler. var Handler = new(HandlerT) + +// Memsize is the memsizeui Handler(?). var Memsize memsizeui.Handler var ( - // Debug Flags + // PProfFlag to enable pprof HTTP server. PProfFlag = cli.BoolFlag{ Name: "pprof", Usage: "Enable the pprof HTTP server", } + // PProfPortFlag to specify HTTP server listening port. PProfPortFlag = cli.IntFlag{ Name: "pprofport", Usage: "pprof HTTP server listening port", Value: 6060, } + // PProfAddrFlag to specify HTTP server address. PProfAddrFlag = cli.StringFlag{ Name: "pprofaddr", Usage: "pprof HTTP server listening interface", Value: "127.0.0.1", } + // MemProfileRateFlag to specify the mem profiling rate. MemProfileRateFlag = cli.IntFlag{ Name: "memprofilerate", Usage: "Turn on memory profiling with the given rate", Value: runtime.MemProfileRate, } + // CPUProfileFlag to specify where to write the CPU profile. CPUProfileFlag = cli.StringFlag{ Name: "cpuprofile", Usage: "Write CPU profile to the given file", } + // TraceFlag to specify where to write the trace execution profile. TraceFlag = cli.StringFlag{ Name: "trace", Usage: "Write execution trace to the given file", @@ -343,12 +350,12 @@ func Setup(ctx *cli.Context) error { // pprof server if ctx.GlobalBool(PProfFlag.Name) { address := fmt.Sprintf("%s:%d", ctx.GlobalString(PProfAddrFlag.Name), ctx.GlobalInt(PProfPortFlag.Name)) - StartPProf(address) + startPProf(address) } return nil } -func StartPProf(address string) { +func startPProf(address string) { http.Handle("/memsize/", http.StripPrefix("/memsize", &Memsize)) log.Info("Starting pprof server", "addr", fmt.Sprintf("http://%s/debug/pprof", address)) go func() { diff --git a/shared/legacyutil/convert_transaction.go b/shared/legacyutil/convert_transaction.go index e6f2a8bad..4e9fb5c6b 100644 --- a/shared/legacyutil/convert_transaction.go +++ b/shared/legacyutil/convert_transaction.go @@ -1,4 +1,4 @@ -// This package exists to convert Ethererum 2.0 types to go-ethereum or +// Package legacyutil exists to convert Ethererum 2.0 types to go-ethereum or // Ethereum 1.0 types. package legacyutil diff --git a/shared/marshal.go b/shared/marshal.go index 1aa99bfcf..f4afef23f 100644 --- a/shared/marshal.go +++ b/shared/marshal.go @@ -153,7 +153,7 @@ func Deserialize(data []byte) ([]RawBlob, error) { // if indicator is non-terminal, increase partitions counter if databyteLength == 0 { - numPartitions += 1 + numPartitions++ } else { // if indicator is terminal, append blob info and reset partitions counter serializedBlob := SerializedBlob{ diff --git a/shared/p2p/service_test.go b/shared/p2p/service_test.go index 9e0ff96e5..33df573fc 100644 --- a/shared/p2p/service_test.go +++ b/shared/p2p/service_test.go @@ -91,7 +91,7 @@ func TestSubscribeToTopic(t *testing.T) { sub := feed.Subscribe(ch) defer sub.Unsubscribe() - testSubscribe(t, s, gsub, ch, ctx) + testSubscribe(ctx, t, s, gsub, ch) } func TestSubscribe(t *testing.T) { @@ -116,10 +116,10 @@ func TestSubscribe(t *testing.T) { sub := s.Subscribe(pb.CollationBodyRequest{}, ch) defer sub.Unsubscribe() - testSubscribe(t, s, gsub, ch, ctx) + testSubscribe(ctx, t, s, gsub, ch) } -func testSubscribe(t *testing.T, s Server, gsub *floodsub.PubSub, ch chan Message, ctx context.Context) { +func testSubscribe(ctx context.Context, t *testing.T, s Server, gsub *floodsub.PubSub, ch chan Message) { topic := pb.Topic_COLLATION_BODY_REQUEST msgType := topicTypeMapping[topic] go s.subscribeToTopic(topic, msgType)