From 41e12c16c5e4e4906b12eae2a3a6a30029f723ec Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Wed, 24 Mar 2021 13:44:16 +0100 Subject: [PATCH 1/6] make: add Makefile As a preparation to get rid of the clunky to handle goclean.sh, we add a Makefile that behaves in mostly the same way as does lnd's Makefile. --- Makefile | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 Makefile diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..5c2de92 --- /dev/null +++ b/Makefile @@ -0,0 +1,132 @@ +PKG := github.com/btcsuite/btcwallet + +GOVERALLS_PKG := github.com/mattn/goveralls +LINT_PKG := github.com/golangci/golangci-lint/cmd/golangci-lint +GOACC_PKG := github.com/ory/go-acc +GOIMPORTS_PKG := golang.org/x/tools/cmd/goimports + +GO_BIN := ${GOPATH}/bin +GOVERALLS_BIN := $(GO_BIN)/goveralls +LINT_BIN := $(GO_BIN)/golangci-lint +GOACC_BIN := $(GO_BIN)/go-acc + +LINT_COMMIT := v1.18.0 +GOACC_COMMIT := ddc355013f90fea78d83d3a6c71f1d37ac07ecd5 + +DEPGET := cd /tmp && GO111MODULE=on go get -v +GOBUILD := GO111MODULE=on go build -v +GOINSTALL := GO111MODULE=on go install -v +GOTEST := GO111MODULE=on go test + +GOLIST := go list -deps $(PKG)/... | grep '$(PKG)' +GOLIST_COVER := $$(go list -deps $(PKG)/... | grep '$(PKG)') +GOFILES_NOVENDOR = $(shell find . -type f -name '*.go' -not -path "./vendor/*") + +RM := rm -f +CP := cp +MAKE := make +XARGS := xargs -L 1 + +# Linting uses a lot of memory, so keep it under control by limiting the number +# of workers if requested. +ifneq ($(workers),) +LINT_WORKERS = --concurrency=$(workers) +endif + +LINT = $(LINT_BIN) run -v $(LINT_WORKERS) + +GREEN := "\\033[0;32m" +NC := "\\033[0m" +define print + echo $(GREEN)$1$(NC) +endef + +default: build + +all: build check + +# ============ +# DEPENDENCIES +# ============ + +$(GOVERALLS_BIN): + @$(call print, "Fetching goveralls.") + go get -u $(GOVERALLS_PKG) + +$(LINT_BIN): + @$(call print, "Fetching linter") + $(DEPGET) $(LINT_PKG)@$(LINT_COMMIT) + +$(GOACC_BIN): + @$(call print, "Fetching go-acc") + $(DEPGET) $(GOACC_PKG)@$(GOACC_COMMIT) + +goimports: + @$(call print, "Installing goimports.") + $(DEPGET) $(GOIMPORTS_PKG) + +# ============ +# INSTALLATION +# ============ + +build: + @$(call print, "Compiling btcwallet.") + $(GOBUILD) $(PKG)/... + +install: + @$(call print, "Installing btcwallet.") + $(GOINSTALL) $(PKG) + $(GOINSTALL) $(PKG)/cmd/dropwtxmgr + $(GOINSTALL) $(PKG)/cmd/sweepaccount + +# ======= +# TESTING +# ======= + +check: unit + +unit: + @$(call print, "Running unit tests.") + $(GOLIST) | $(XARGS) env $(GOTEST) + +unit-cover: $(GOACC_BIN) + @$(call print, "Running unit coverage tests.") + $(GOACC_BIN) $(GOLIST_COVER) + +unit-race: + @$(call print, "Running unit race tests.") + env CGO_ENABLED=1 GORACE="history_size=7 halt_on_errors=1" $(GOLIST) | $(XARGS) env $(GOTEST) -race + +goveralls: $(GOVERALLS_BIN) + @$(call print, "Sending coverage report.") + $(GOVERALLS_BIN) -coverprofile=coverage.txt -service=travis-ci + +# ========= +# UTILITIES +# ========= + +fmt: goimports + @$(call print, "Fixing imports.") + goimports -w $(GOFILES_NOVENDOR) + @$(call print, "Formatting source.") + gofmt -l -w -s $(GOFILES_NOVENDOR) + +lint: $(LINT_BIN) + @$(call print, "Linting source.") + $(LINT) + +clean: + @$(call print, "Cleaning source.$(NC)") + $(RM) coverage.txt + +.PHONY: all \ + default \ + build \ + check \ + unit \ + unit-cover \ + unit-race \ + goveralls \ + fmt \ + lint \ + clean From 40414fde37ceff426257a4f7f54c47905d0d6d1d Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Wed, 24 Mar 2021 13:45:11 +0100 Subject: [PATCH 2/6] rpc: run make fmt to format all code --- rpc/walletrpc/api.pb.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/rpc/walletrpc/api.pb.go b/rpc/walletrpc/api.pb.go index e6716e4..6c66181 100644 --- a/rpc/walletrpc/api.pb.go +++ b/rpc/walletrpc/api.pb.go @@ -60,12 +60,15 @@ It has these top-level messages: */ package walletrpc -import proto "github.com/golang/protobuf/proto" -import fmt "fmt" -import math "math" - import ( + fmt "fmt" + + proto "github.com/golang/protobuf/proto" + + math "math" + context "golang.org/x/net/context" + grpc "google.golang.org/grpc" ) From ac3ec4fa032f641005be6eb8b15c7f321f01caec Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Wed, 24 Mar 2021 13:49:55 +0100 Subject: [PATCH 3/6] lint: add new linter configuration We use the golangci-lint in lnd and it has quite a few more detections enabled than what's in the current gotest.sh script. We don't start with a given base commit on purpose but instead fix everything the linter finds in the following commits. --- .golangci.yml | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..4c7f466 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,54 @@ +run: + # timeout for analysis + deadline: 10m + +linters-settings: + govet: + # Don't report about shadowed variables + check-shadowing: false + gofmt: + # simplify code: gofmt with `-s` option, true by default + simplify: true + +linters: + enable-all: true + disable: + # Global variables are used in many places throughout the code base. + - gochecknoglobals + + # Some lines are over 80 characters on purpose and we don't want to make them + # even longer by marking them as 'nolint'. + - lll + + # We don't care (enough) about misaligned structs to lint that. + - maligned + + # We have long functions, especially in tests. Moving or renaming those would + # trigger funlen problems that we may not want to solve at that time. + - funlen + + # Disable for now as we haven't yet tuned the sensitivity to our codebase + # yet. Enabling by default for example, would also force new contributors to + # potentially extensively refactor code, when they want to smaller change to + # land. + - gocyclo + + # Instances of table driven tests that don't pre-allocate shouldn't trigger + # the linter. + - prealloc + + # Init functions are used by loggers throughout the codebase. + - gochecknoinits + + # Explicit types are okay. + - interfacer + +issues: + exclude-rules: + # Exclude gosec from running for tests so that tests with weak randomness + # (math/rand) will pass the linter. + - path: _test\.go + linters: + - gosec + - errcheck + - dupl From 3a5d9f84b0bc512c9caf7d4ac7c2bf0b45b059d5 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Wed, 24 Mar 2021 14:43:24 +0100 Subject: [PATCH 4/6] multi: fix linter issues --- btcwallet.go | 2 +- chain/bitcoind_client.go | 15 +++-- chain/log.go | 26 ------- chain/neutrino.go | 15 +++-- chain/rpc.go | 12 ++-- cmd/sweepaccount/main.go | 5 +- config.go | 28 ++++---- internal/cfgutil/amount.go | 4 +- internal/legacy/keystore/keystore.go | 25 ++----- internal/legacy/keystore/keystore_test.go | 12 ++-- internal/prompt/prompt.go | 10 +-- log.go | 15 +---- rpc/legacyrpc/errors.go | 2 +- rpc/legacyrpc/methods.go | 66 +++++++----------- rpc/legacyrpc/rpcserver_test.go | 1 + rpc/legacyrpc/server.go | 34 ++-------- rpc/rpcserver/log.go | 2 +- rpc/rpcserver/server.go | 82 ++++++++--------------- snacl/snacl.go | 2 +- waddrmgr/address.go | 4 +- waddrmgr/db.go | 51 +++----------- waddrmgr/log.go | 17 ----- waddrmgr/manager.go | 31 +++------ waddrmgr/manager_test.go | 59 ++++++---------- waddrmgr/migrations.go | 2 +- waddrmgr/migrations_test.go | 5 +- waddrmgr/scoped_manager.go | 18 ++--- wallet/chainntfns.go | 6 +- wallet/createtx.go | 4 +- wallet/import.go | 2 +- wallet/import_test.go | 2 - wallet/log.go | 17 ----- wallet/multisig.go | 2 +- wallet/notifications.go | 26 +------ wallet/recovery_test.go | 2 +- wallet/signer_test.go | 1 + wallet/unstable.go | 4 +- wallet/wallet.go | 48 +++---------- wallet/wallet_test.go | 1 + walletsetup.go | 23 +++---- 40 files changed, 221 insertions(+), 462 deletions(-) diff --git a/btcwallet.go b/btcwallet.go index 72728fb..6974fa7 100644 --- a/btcwallet.go +++ b/btcwallet.go @@ -166,11 +166,11 @@ func rpcClientConnectLoop(legacyRPCServer *legacyrpc.Server, loader *wallet.Load "bdb", filepath.Join(netDir, "neutrino.db"), true, cfg.DBTimeout, ) - defer spvdb.Close() if err != nil { log.Errorf("Unable to create Neutrino DB: %s", err) continue } + defer spvdb.Close() chainService, err = neutrino.NewChainService( neutrino.Config{ DataDir: netDir, diff --git a/chain/bitcoind_client.go b/chain/bitcoind_client.go index a8bcc1e..b7f870f 100644 --- a/chain/bitcoind_client.go +++ b/chain/bitcoind_client.go @@ -221,7 +221,7 @@ func (c *BitcoindClient) Notifications() <-chan interface{} { // // NOTE: This is part of the chain.Interface interface. func (c *BitcoindClient) NotifyReceived(addrs []btcutil.Address) error { - c.NotifyBlocks() + _ = c.NotifyBlocks() select { case c.rescanUpdate <- addrs: @@ -235,7 +235,7 @@ func (c *BitcoindClient) NotifyReceived(addrs []btcutil.Address) error { // NotifySpent allows the chain backend to notify the caller whenever a // transaction spends any of the given outpoints. func (c *BitcoindClient) NotifySpent(outPoints []*wire.OutPoint) error { - c.NotifyBlocks() + _ = c.NotifyBlocks() select { case c.rescanUpdate <- outPoints: @@ -249,7 +249,7 @@ func (c *BitcoindClient) NotifySpent(outPoints []*wire.OutPoint) error { // NotifyTx allows the chain backend to notify the caller whenever any of the // given transactions confirm within the chain. func (c *BitcoindClient) NotifyTx(txids []chainhash.Hash) error { - c.NotifyBlocks() + _ = c.NotifyBlocks() select { case c.rescanUpdate <- txids: @@ -371,6 +371,8 @@ func (c *BitcoindClient) RescanBlocks( rescannedBlocks := make([]btcjson.RescannedBlock, 0, len(blockHashes)) for _, hash := range blockHashes { + hash := hash + header, err := c.GetBlockHeaderVerbose(&hash) if err != nil { log.Warnf("Unable to get header %s from bitcoind: %s", @@ -618,7 +620,7 @@ func (c *BitcoindClient) ntfnHandler() { newBlockHeight := bestBlock.Height + 1 _ = c.filterBlock(newBlock, newBlockHeight, true) - // With the block succesfully filtered, we'll + // With the block successfully filtered, we'll // make it our new best block. bestBlock.Hash = newBlock.BlockHash() bestBlock.Height = newBlockHeight @@ -847,10 +849,11 @@ func (c *BitcoindClient) reorg(currentBlock waddrmgr.BlockStamp, // Our current block should now reflect the previous one to // continue the common ancestor search. - currentHeader, err = c.GetBlockHeader(¤tHeader.PrevBlock) + prevBlock := ¤tHeader.PrevBlock + currentHeader, err = c.GetBlockHeader(prevBlock) if err != nil { return fmt.Errorf("unable to get block header for %v: %v", - currentHeader.PrevBlock, err) + prevBlock, err) } currentBlock.Height-- diff --git a/chain/log.go b/chain/log.go index d99b284..d336eac 100644 --- a/chain/log.go +++ b/chain/log.go @@ -28,29 +28,3 @@ func DisableLog() { func UseLogger(logger btclog.Logger) { log = logger } - -// LogClosure is a closure that can be printed with %v to be used to -// generate expensive-to-create data for a detailed log level and avoid doing -// the work if the data isn't printed. -type logClosure func() string - -// String invokes the log closure and returns the results string. -func (c logClosure) String() string { - return c() -} - -// newLogClosure returns a new closure over the passed function which allows -// it to be used as a parameter in a logging function that is only invoked when -// the logging level is such that the message will actually be logged. -func newLogClosure(c func() string) logClosure { - return logClosure(c) -} - -// pickNoun returns the singular or plural form of a noun depending -// on the count n. -func pickNoun(n int, singular, plural string) string { - if n == 1 { - return singular - } - return plural -} diff --git a/chain/neutrino.go b/chain/neutrino.go index 28987f7..cc5dde1 100644 --- a/chain/neutrino.go +++ b/chain/neutrino.go @@ -66,7 +66,10 @@ func (s *NeutrinoClient) BackEnd() string { // Start replicates the RPC client's Start method. func (s *NeutrinoClient) Start() error { - s.CS.Start() + if err := s.CS.Start(); err != nil { + return fmt.Errorf("error starting chain service: %v", err) + } + s.clientMtx.Lock() defer s.clientMtx.Unlock() if !s.started { @@ -178,8 +181,8 @@ func (s *NeutrinoClient) SendRawTransaction(tx *wire.MsgTx, allowHighFees bool) // FilterBlocks scans the blocks contained in the FilterBlocksRequest for any // addresses of interest. For each requested block, the corresponding compact // filter will first be checked for matches, skipping those that do not report -// anything. If the filter returns a postive match, the full block will be -// fetched and filtered. This method returns a FilterBlocksReponse for the first +// anything. If the filter returns a positive match, the full block will be +// fetched and filtered. This method returns a FilterBlocksResponse for the first // block containing a matching address. If no matches are found in the range of // blocks requested, the returned response will be nil. func (s *NeutrinoClient) FilterBlocks( @@ -361,11 +364,11 @@ func (s *NeutrinoClient) Rescan(startHash *chainhash.Hash, addrs []btcutil.Addre bestBlock, err := s.CS.BestBlock() if err != nil { - return fmt.Errorf("Can't get chain service's best block: %s", err) + return fmt.Errorf("can't get chain service's best block: %s", err) } header, err := s.CS.GetBlockHeader(&bestBlock.Hash) if err != nil { - return fmt.Errorf("Can't get block header for hash %v: %s", + return fmt.Errorf("can't get block header for hash %v: %s", bestBlock.Hash, err) } @@ -384,7 +387,7 @@ func (s *NeutrinoClient) Rescan(startHash *chainhash.Hash, addrs []btcutil.Addre select { case s.enqueueNotification <- &RescanFinished{ Hash: startHash, - Height: int32(bestBlock.Height), + Height: bestBlock.Height, Time: header.Timestamp, }: case <-s.quit: diff --git a/chain/rpc.go b/chain/rpc.go index af9bbf5..eafb63e 100644 --- a/chain/rpc.go +++ b/chain/rpc.go @@ -154,8 +154,8 @@ func (c *RPCClient) IsCurrent() bool { return bestHeader.Timestamp.After(time.Now().Add(-isCurrentDelta)) } -// Rescan wraps the normal Rescan command with an additional paramter that -// allows us to map an oupoint to the address in the chain that it pays to. +// Rescan wraps the normal Rescan command with an additional parameter that +// allows us to map an outpoint to the address in the chain that it pays to. // This is useful when using BIP 158 filters as they include the prev pkScript // rather than the full outpoint. func (c *RPCClient) Rescan(startHash *chainhash.Hash, addrs []btcutil.Address, @@ -163,10 +163,12 @@ func (c *RPCClient) Rescan(startHash *chainhash.Hash, addrs []btcutil.Address, flatOutpoints := make([]*wire.OutPoint, 0, len(outPoints)) for ops := range outPoints { + ops := ops + flatOutpoints = append(flatOutpoints, &ops) } - return c.Client.Rescan(startHash, addrs, flatOutpoints) + return c.Client.Rescan(startHash, addrs, flatOutpoints) // nolint:staticcheck } // WaitForShutdown blocks until both the client has finished disconnecting @@ -198,8 +200,8 @@ func (c *RPCClient) BlockStamp() (*waddrmgr.BlockStamp, error) { // FilterBlocks scans the blocks contained in the FilterBlocksRequest for any // addresses of interest. For each requested block, the corresponding compact // filter will first be checked for matches, skipping those that do not report -// anything. If the filter returns a postive match, the full block will be -// fetched and filtered. This method returns a FilterBlocksReponse for the first +// anything. If the filter returns a positive match, the full block will be +// fetched and filtered. This method returns a FilterBlocksResponse for the first // block containing a matching address. If no matches are found in the range of // blocks requested, the returned response will be nil. func (c *RPCClient) FilterBlocks( diff --git a/cmd/sweepaccount/main.go b/cmd/sweepaccount/main.go index 9ec6ba0..c27ca8f 100644 --- a/cmd/sweepaccount/main.go +++ b/cmd/sweepaccount/main.go @@ -146,6 +146,8 @@ func makeInputSource(outputs []btcjson.ListUnspentResult) txauthor.InputSource { sourceErr error ) for _, output := range outputs { + output := output + outputAmount, err := btcutil.NewAmount(output.Amount) if err != nil { sourceErr = fmt.Errorf( @@ -315,7 +317,8 @@ func sweep() error { totalSwept, numPublished, transactionNoun) } if numErrors > 0 { - return fmt.Errorf("Failed to publish %d %s", numErrors, transactionNoun) + return fmt.Errorf("failed to publish %d %s", numErrors, + transactionNoun) } return nil diff --git a/config.go b/config.go index 7c80280..eccf4b7 100644 --- a/config.go +++ b/config.go @@ -198,7 +198,7 @@ func parseAndSetDebugLevels(debugLevel string) error { if !strings.Contains(debugLevel, ",") && !strings.Contains(debugLevel, "=") { // Validate debug log level. if !validLogLevel(debugLevel) { - str := "The specified debug level [%v] is invalid" + str := "the specified debug level [%v] is invalid" return fmt.Errorf(str, debugLevel) } @@ -212,7 +212,7 @@ func parseAndSetDebugLevels(debugLevel string) error { // issues and update the log levels accordingly. for _, logLevelPair := range strings.Split(debugLevel, ",") { if !strings.Contains(logLevelPair, "=") { - str := "The specified debug level contains an invalid " + + str := "the specified debug level contains an invalid " + "subsystem/level pair [%v]" return fmt.Errorf(str, logLevelPair) } @@ -223,14 +223,14 @@ func parseAndSetDebugLevels(debugLevel string) error { // Validate subsystem. if _, exists := subsystemLoggers[subsysID]; !exists { - str := "The specified subsystem [%v] is invalid -- " + + str := "the specified subsystem [%v] is invalid -- " + "supported subsytems %v" return fmt.Errorf(str, subsysID, supportedSubsystems()) } // Validate log level. if !validLogLevel(logLevel) { - str := "The specified debug level [%v] is invalid" + str := "the specified debug level [%v] is invalid" return fmt.Errorf(str, logLevel) } @@ -418,8 +418,8 @@ func loadConfig() (*config, []string, error) { dbPath := filepath.Join(netDir, wallet.WalletDBName) if cfg.CreateTemp && cfg.Create { - err := fmt.Errorf("The flags --create and --createtemp can not " + - "be specified together. Use --help for more information.") + err := fmt.Errorf("the flags --create and --createtemp can not " + + "be specified together. Use --help for more information") fmt.Fprintln(os.Stderr, err) return nil, nil, err } @@ -430,7 +430,7 @@ func loadConfig() (*config, []string, error) { return nil, nil, err } - if cfg.CreateTemp { + if cfg.CreateTemp { // nolint:gocritic tempWalletExists := false if dbFileExists { @@ -457,8 +457,8 @@ func loadConfig() (*config, []string, error) { // Error if the create flag is set and the wallet already // exists. if dbFileExists { - err := fmt.Errorf("The wallet database file `%v` "+ - "already exists.", dbPath) + err := fmt.Errorf("the wallet database file `%v` "+ + "already exists", dbPath) fmt.Fprintln(os.Stderr, err) return nil, nil, err } @@ -485,11 +485,11 @@ func loadConfig() (*config, []string, error) { return nil, nil, err } if !keystoreExists { - err = fmt.Errorf("The wallet does not exist. Run with the " + - "--create option to initialize and create it.") + err = fmt.Errorf("the wallet does not exist, run with " + + "the --create option to initialize and create it") } else { - err = fmt.Errorf("The wallet is in legacy format. Run with the " + - "--create option to import it.") + err = fmt.Errorf("the wallet is in legacy format, run " + + "with the --create option to import it") } fmt.Fprintln(os.Stderr, err) return nil, nil, err @@ -605,7 +605,7 @@ func loadConfig() (*config, []string, error) { for _, addr := range cfg.ExperimentalRPCListeners { _, seen := seenAddresses[addr] if seen { - err := fmt.Errorf("Address `%s` may not be "+ + err := fmt.Errorf("address `%s` may not be "+ "used as a listener address for both "+ "RPC servers", addr) fmt.Fprintln(os.Stderr, err) diff --git a/internal/cfgutil/amount.go b/internal/cfgutil/amount.go index 5fa9c0c..2e8aaf8 100644 --- a/internal/cfgutil/amount.go +++ b/internal/cfgutil/amount.go @@ -22,12 +22,12 @@ func NewAmountFlag(defaultValue btcutil.Amount) *AmountFlag { return &AmountFlag{defaultValue} } -// MarshalFlag satisifes the flags.Marshaler interface. +// MarshalFlag satisfies the flags.Marshaler interface. func (a *AmountFlag) MarshalFlag() (string, error) { return a.Amount.String(), nil } -// UnmarshalFlag satisifes the flags.Unmarshaler interface. +// UnmarshalFlag satisfies the flags.Unmarshaler interface. func (a *AmountFlag) UnmarshalFlag(value string) error { value = strings.TrimSuffix(value, " BTC") valueF64, err := strconv.ParseFloat(value, 64) diff --git a/internal/legacy/keystore/keystore.go b/internal/legacy/keystore/keystore.go index f671f18..c1430e3 100644 --- a/internal/legacy/keystore/keystore.go +++ b/internal/legacy/keystore/keystore.go @@ -38,10 +38,6 @@ const ( // Length in bytes of KDF output. kdfOutputBytes = 32 - - // Maximum length in bytes of a comment that can have a size represented - // as a uint16. - maxCommentLen = (1 << 16) - 1 ) const ( @@ -66,9 +62,9 @@ var fileID = [8]byte{0xba, 'W', 'A', 'L', 'L', 'E', 'T', 0x00} type entryHeader byte const ( - addrCommentHeader entryHeader = 1 << iota - txCommentHeader - deletedHeader + addrCommentHeader entryHeader = 1 << iota //nolint:varcheck,deadcode,unused + txCommentHeader // nolint:varcheck,deadcode,unused + deletedHeader // nolint:varcheck,deadcode,unused scriptHeader addrHeader entryHeader = 0 ) @@ -233,9 +229,6 @@ func chainedPubKey(pubkey, chaincode []byte) ([]byte, error) { return nil, err } newX, newY := btcec.S256().ScalarMult(oldPk.X, oldPk.Y, xorbytes) - if err != nil { - return nil, err - } newPk := &btcec.PublicKey{ Curve: btcec.S256(), X: newX, @@ -501,9 +494,6 @@ func (net *netParams) WriteTo(w io.Writer) (int64, error) { // Stringified byte slices for use as map lookup keys. type addressKey string -type transactionHashKey string - -type comment []byte func getAddressKey(addr btcutil.Address) addressKey { return addressKey(addr.ScriptAddress()) @@ -775,7 +765,7 @@ func (s *Store) writeTo(w io.Writer) (n int64, err error) { importedAddrs = append(importedAddrs, e) } } - wts = append(chainedAddrs, importedAddrs...) + wts = append(chainedAddrs, importedAddrs...) // nolint:gocritic appendedEntries := varEntries{store: s, entries: wts} // Iterate through each entry needing to be written. If data @@ -1376,7 +1366,7 @@ func (s *Store) SyncedTo() (hash *chainhash.Hash, height int32) { } } } - return + return // nolint:nakedret } // NewIterateRecentBlocks returns an iterator for recently-seen blocks. @@ -2108,7 +2098,7 @@ func (k *publicKey) ReadFrom(r io.Reader) (n int64, err error) { n += read *k = append([]byte{format}, s...) - return + return // nolint:nakedret } func (k *publicKey) WriteTo(w io.Writer) (n int64, err error) { @@ -2595,8 +2585,7 @@ func (a *btcAddress) ExportPrivKey() (*btcutil.WIF, error) { // as our program's assumptions are so broken that this needs to be // caught immediately, and a stack trace here is more useful than // elsewhere. - wif, err := btcutil.NewWIF((*btcec.PrivateKey)(pk), a.store.netParams(), - a.Compressed()) + wif, err := btcutil.NewWIF(pk, a.store.netParams(), a.Compressed()) if err != nil { panic(err) } diff --git a/internal/legacy/keystore/keystore_test.go b/internal/legacy/keystore/keystore_test.go index 2119e4f..3ef15e4 100644 --- a/internal/legacy/keystore/keystore_test.go +++ b/internal/legacy/keystore/keystore_test.go @@ -703,7 +703,7 @@ func TestImportPrivateKey(t *testing.T) { } // import priv key - wif, err := btcutil.NewWIF((*btcec.PrivateKey)(pk), tstNetParams, false) + wif, err := btcutil.NewWIF(pk, tstNetParams, false) if err != nil { t.Fatal(err) } @@ -761,8 +761,8 @@ func TestImportPrivateKey(t *testing.T) { return } - // Mark imported address as partially synced with a block somewhere inbetween - // the import height and the chain height. + // Mark imported address as partially synced with a block somewhere in + // between the import height and the chain height. partialHeight := (createHeight-importHeight)/2 + importHeight if err := w2.SetSyncStatus(address, PartialSync(partialHeight)); err != nil { t.Errorf("Cannot mark address partially synced: %v", err) @@ -1037,7 +1037,7 @@ func TestImportScript(t *testing.T) { return } - if sinfo.RequiredSigs() != sinfo.RequiredSigs() { + if sinfo.RequiredSigs() != sinfo2.RequiredSigs() { t.Errorf("original and serailised scriptinfo requiredsigs "+ "don't match %d != %d", sinfo.RequiredSigs(), sinfo2.RequiredSigs()) @@ -1063,8 +1063,8 @@ func TestImportScript(t *testing.T) { return } - // Mark imported address as partially synced with a block somewhere inbetween - // the import height and the chain height. + // Mark imported address as partially synced with a block somewhere in + // between the import height and the chain height. partialHeight := (createHeight-importHeight)/2 + importHeight if err := w2.SetSyncStatus(address, PartialSync(partialHeight)); err != nil { t.Errorf("Cannot mark address partially synced: %v", err) diff --git a/internal/prompt/prompt.go b/internal/prompt/prompt.go index 571ebfa..b967803 100644 --- a/internal/prompt/prompt.go +++ b/internal/prompt/prompt.go @@ -100,8 +100,10 @@ func promptList(reader *bufio.Reader, prefix string, validResponses []string, de // promptListBool prompts the user for a boolean (yes/no) with the given prefix. // The function will repeat the prompt to the user until they enter a valid -// reponse. -func promptListBool(reader *bufio.Reader, prefix string, defaultEntry string) (bool, error) { +// response. +func promptListBool(reader *bufio.Reader, prefix string, + defaultEntry string) (bool, error) { // nolint:unparam + // Setup the valid responses. valid := []string{"n", "no", "y", "yes"} response, err := promptList(reader, prefix, valid, defaultEntry) @@ -114,7 +116,7 @@ func promptListBool(reader *bufio.Reader, prefix string, defaultEntry string) (b // promptPass prompts the user for a passphrase with the given prefix. The // function will ask the user to confirm the passphrase and will repeat the // prompts until they enter a matching response. -func promptPass(reader *bufio.Reader, prefix string, confirm bool) ([]byte, error) { +func promptPass(_ *bufio.Reader, prefix string, confirm bool) ([]byte, error) { // Prompt the user until they enter a passphrase. prompt := fmt.Sprintf("%s: ", prefix) for { @@ -177,7 +179,7 @@ func PrivatePass(reader *bufio.Reader, legacyKeyStore *keystore.Store) ([]byte, } // Keep prompting the user until the passphrase is correct. - if err := legacyKeyStore.Unlock([]byte(privPass)); err != nil { + if err := legacyKeyStore.Unlock(privPass); err != nil { if err == keystore.ErrWrongPassphrase { fmt.Println(err) continue diff --git a/log.go b/log.go index 0c866d2..a441889 100644 --- a/log.go +++ b/log.go @@ -26,8 +26,8 @@ import ( type logWriter struct{} func (logWriter) Write(p []byte) (n int, err error) { - os.Stdout.Write(p) - logRotatorPipe.Write(p) + _, _ = os.Stdout.Write(p) + _, _ = logRotatorPipe.Write(p) return len(p), nil } @@ -101,7 +101,7 @@ func initLogRotator(logFile string) { } pr, pw := io.Pipe() - go r.Run(pr) + go func() { _ = r.Run(pr) }() logRotator = r logRotatorPipe = pw @@ -132,12 +132,3 @@ func setLogLevels(logLevel string) { setLogLevel(subsystemID, logLevel) } } - -// pickNoun returns the singular or plural form of a noun depending -// on the count n. -func pickNoun(n int, singular, plural string) string { - if n == 1 { - return singular - } - return plural -} diff --git a/rpc/legacyrpc/errors.go b/rpc/legacyrpc/errors.go index 2116572..911bd7f 100644 --- a/rpc/legacyrpc/errors.go +++ b/rpc/legacyrpc/errors.go @@ -11,7 +11,7 @@ import ( ) // TODO(jrick): There are several error paths which 'replace' various errors -// with a more appropiate error from the btcjson package. Create a map of +// with a more appropriate error from the btcjson package. Create a map of // these replacements so they can be handled once after an RPC handler has // returned and before the error is marshaled. diff --git a/rpc/legacyrpc/methods.go b/rpc/legacyrpc/methods.go index bd942a9..fff9d59 100644 --- a/rpc/legacyrpc/methods.go +++ b/rpc/legacyrpc/methods.go @@ -30,11 +30,10 @@ import ( "github.com/btcsuite/btcwallet/wtxmgr" ) -// confirmed checks whether a transaction at height txHeight has met minconf -// confirmations for a blockchain at height curHeight. -func confirmed(minconf, txHeight, curHeight int32) bool { - return confirms(txHeight, curHeight) >= minconf -} +const ( + // defaultAccountName is the name of the wallet's default account. + defaultAccountName = "default" +) // confirms returns the number of confirmations for a transaction in a block at // height txHeight (or -1 for an unconfirmed tx) given the chain height @@ -51,7 +50,7 @@ func confirms(txHeight, curHeight int32) int32 { // requestHandler is a handler function to handle an unmarshaled and parsed // request into a marshalable response. If the error is a *btcjson.RPCError // or any of the above special error classes, the server will respond with -// the JSON-RPC appropiate error code. All other errors use the wallet +// the JSON-RPC appropriate error code. All other errors use the wallet // catch-all error code, btcjson.ErrRPCWallet. type requestHandler func(interface{}, *wallet.Wallet) (interface{}, error) @@ -139,7 +138,7 @@ var rpcHandlers = map[string]struct { } // unimplemented handles an unimplemented RPC request with the -// appropiate error. +// appropriate error. func unimplemented(interface{}, *wallet.Wallet) (interface{}, error) { return nil, &btcjson.RPCError{ Code: btcjson.ErrRPCUnimplemented, @@ -274,8 +273,7 @@ func jsonError(err error) *btcjson.RPCError { case ParseError: code = btcjson.ErrRPCParse.Code case waddrmgr.ManagerError: - switch e.ErrorCode { - case waddrmgr.ErrWrongPassphrase: + if e.ErrorCode == waddrmgr.ErrWrongPassphrase { code = btcjson.ErrRPCWalletPassphraseIncorrect } } @@ -375,7 +373,7 @@ func createMultiSig(icmd interface{}, w *wallet.Wallet) (interface{}, error) { } // dumpPrivKey handles a dumpprivkey request with the private key -// for a single address, or an appropiate error if the wallet +// for a single address, or an appropriate error if the wallet // is locked. func dumpPrivKey(icmd interface{}, w *wallet.Wallet) (interface{}, error) { cmd := icmd.(*btcjson.DumpPrivKeyCmd) @@ -394,18 +392,6 @@ func dumpPrivKey(icmd interface{}, w *wallet.Wallet) (interface{}, error) { return key, err } -// dumpWallet handles a dumpwallet request by returning all private -// keys in a wallet, or an appropiate error if the wallet is locked. -// TODO: finish this to match bitcoind by writing the dump to a file. -func dumpWallet(icmd interface{}, w *wallet.Wallet) (interface{}, error) { - keys, err := w.DumpPrivKeys() - if waddrmgr.IsError(err, waddrmgr.ErrLocked) { - return nil, &ErrWalletUnlockNeeded - } - - return keys, err -} - // getAddressesByAccount handles a getaddressesbyaccount request by returning // all addresses for an account, or an error if the requested account does // not exist. @@ -584,7 +570,7 @@ func getAccountAddress(icmd interface{}, w *wallet.Wallet) (interface{}, error) func getUnconfirmedBalance(icmd interface{}, w *wallet.Wallet) (interface{}, error) { cmd := icmd.(*btcjson.GetUnconfirmedBalanceCmd) - acctName := "default" + acctName := defaultAccountName if cmd.Account != nil { acctName = *cmd.Account } @@ -669,7 +655,7 @@ func createNewAccount(icmd interface{}, w *wallet.Wallet) (interface{}, error) { } // renameAccount handles a renameaccount request by renaming an account. -// If the account does not exist an appropiate error will be returned. +// If the account does not exist an appropriate error will be returned. func renameAccount(icmd interface{}, w *wallet.Wallet) (interface{}, error) { cmd := icmd.(*btcjson.RenameAccountCmd) @@ -688,14 +674,14 @@ func renameAccount(icmd interface{}, w *wallet.Wallet) (interface{}, error) { } // getNewAddress handles a getnewaddress request by returning a new -// address for an account. If the account does not exist an appropiate +// address for an account. If the account does not exist an appropriate // error is returned. // TODO: Follow BIP 0044 and warn if number of unused addresses exceeds // the gap limit. func getNewAddress(icmd interface{}, w *wallet.Wallet) (interface{}, error) { cmd := icmd.(*btcjson.GetNewAddressCmd) - acctName := "default" + acctName := defaultAccountName if cmd.Account != nil { acctName = *cmd.Account } @@ -720,7 +706,7 @@ func getNewAddress(icmd interface{}, w *wallet.Wallet) (interface{}, error) { func getRawChangeAddress(icmd interface{}, w *wallet.Wallet) (interface{}, error) { cmd := icmd.(*btcjson.GetRawChangeAddressCmd) - acctName := "default" + acctName := defaultAccountName if cmd.Account != nil { acctName = *cmd.Account } @@ -926,8 +912,8 @@ func getTransaction(icmd interface{}, w *wallet.Wallet) (interface{}, error) { // localeHelpDescs maps from locale strings (e.g. "en_US") to a function that // builds a map of help texts for each RPC server method. This prevents help // text maps for every locale map from being rooted and created during init. -// Instead, the appropiate function is looked up when help text is first needed -// using the current locale and saved to the global below for futher reuse. +// Instead, the appropriate function is looked up when help text is first needed +// using the current locale and saved to the global below for further reuse. // // requestUsages contains single line usages for every supported request, // separated by newlines. It is set during init. These usages are used for all @@ -958,11 +944,11 @@ func helpNoChainRPC(icmd interface{}, w *wallet.Wallet) (interface{}, error) { // methods, or full help for a specific method. The chainClient is optional, // and this is simply a helper function for the HelpNoChainRPC and // HelpWithChainRPC handlers. -func help(icmd interface{}, w *wallet.Wallet, chainClient *chain.RPCClient) (interface{}, error) { +func help(icmd interface{}, _ *wallet.Wallet, chainClient *chain.RPCClient) (interface{}, error) { cmd := icmd.(*btcjson.HelpCmd) // btcd returns different help messages depending on the kind of - // connection the client is using. Only methods availble to HTTP POST + // connection the client is using. Only methods available to HTTP POST // clients are available to be used by wallet clients, even though // wallet itself is a websocket client to btcd. Therefore, create a // POST client as needed. @@ -1177,7 +1163,7 @@ func listReceivedByAddress(icmd interface{}, w *wallet.Wallet) (interface{}, err // Massage address data into output format. numAddresses := len(allAddrData) - ret := make([]btcjson.ListReceivedByAddressResult, numAddresses, numAddresses) + ret := make([]btcjson.ListReceivedByAddressResult, numAddresses) idx := 0 for address, addrData := range allAddrData { ret[idx] = btcjson.ListReceivedByAddressResult{ @@ -1185,6 +1171,7 @@ func listReceivedByAddress(icmd interface{}, w *wallet.Wallet) (interface{}, err Amount: addrData.amount.ToBTC(), Confirmations: uint64(addrData.confirmations), TxIDs: addrData.tx, + Account: addrData.account, } idx++ } @@ -1386,8 +1373,7 @@ func sendPairs(w *wallet.Wallet, amounts map[string]btcutil.Amount, if waddrmgr.IsError(err, waddrmgr.ErrLocked) { return "", &ErrWalletUnlockNeeded } - switch err.(type) { - case btcjson.RPCError: + if _, ok := err.(btcjson.RPCError); ok { return "", err } @@ -1558,8 +1544,8 @@ func signMessage(icmd interface{}, w *wallet.Wallet) (interface{}, error) { } var buf bytes.Buffer - wire.WriteVarString(&buf, 0, "Bitcoin Signed Message:\n") - wire.WriteVarString(&buf, 0, cmd.Message) + _ = wire.WriteVarString(&buf, 0, "Bitcoin Signed Message:\n") + _ = wire.WriteVarString(&buf, 0, cmd.Message) messageHash := chainhash.DoubleHashB(buf.Bytes()) sigbytes, err := btcec.SignCompact(btcec.S256(), privKey, messageHash, true) @@ -1600,7 +1586,7 @@ func signRawTransaction(icmd interface{}, w *wallet.Wallet, chainClient *chain.R case "SINGLE|ANYONECANPAY": hashType = txscript.SigHashSingle | txscript.SigHashAnyOneCanPay default: - e := errors.New("Invalid sighash parameter") + e := errors.New("invalid sighash parameter") return nil, InvalidParameterError{e} } @@ -1719,7 +1705,7 @@ func signRawTransaction(icmd interface{}, w *wallet.Wallet, chainClient *chain.R var buf bytes.Buffer buf.Grow(tx.SerializeSize()) - // All returned errors (not OOM, which panics) encounted during + // All returned errors (not OOM, which panics) encountered during // bytes.Buffer writes are unexpected. if err = tx.Serialize(&buf); err != nil { panic(err) @@ -1845,8 +1831,8 @@ func verifyMessage(icmd interface{}, w *wallet.Wallet) (interface{}, error) { // Validate the signature - this just shows that it was valid at all. // we will compare it with the key next. var buf bytes.Buffer - wire.WriteVarString(&buf, 0, "Bitcoin Signed Message:\n") - wire.WriteVarString(&buf, 0, cmd.Message) + _ = wire.WriteVarString(&buf, 0, "Bitcoin Signed Message:\n") + _ = wire.WriteVarString(&buf, 0, cmd.Message) expectedMessageHash := chainhash.DoubleHashB(buf.Bytes()) pk, wasCompressed, err := btcec.RecoverCompact(btcec.S256(), sig, expectedMessageHash) diff --git a/rpc/legacyrpc/rpcserver_test.go b/rpc/legacyrpc/rpcserver_test.go index b41ee1a..0eb0d33 100644 --- a/rpc/legacyrpc/rpcserver_test.go +++ b/rpc/legacyrpc/rpcserver_test.go @@ -30,6 +30,7 @@ func TestThrottle(t *testing.T) { return } codes <- res.StatusCode + _ = res.Body.Close() }() } diff --git a/rpc/legacyrpc/server.go b/rpc/legacyrpc/server.go index 60ce593..3c7517b 100644 --- a/rpc/legacyrpc/server.go +++ b/rpc/legacyrpc/server.go @@ -10,7 +10,6 @@ import ( "encoding/base64" "encoding/json" "errors" - "fmt" "io" "io/ioutil" "net" @@ -58,12 +57,11 @@ func (c *websocketClient) send(b []byte) error { // Server holds the items the RPC server may need to access (auth, // config, shutdown, etc.) type Server struct { - httpServer http.Server - wallet *wallet.Wallet - walletLoader *wallet.Loader - chainClient chain.Interface - handlerLookup func(string) (requestHandler, bool) - handlerMu sync.Mutex + httpServer http.Server + wallet *wallet.Wallet + walletLoader *wallet.Loader + chainClient chain.Interface + handlerMu sync.Mutex listeners []net.Listener authsha [sha256.Size]byte @@ -326,7 +324,7 @@ func throttled(threshold int64, h http.Handler) http.Handler { if current-1 >= threshold { log.Warnf("Reached threshold of %d concurrent active clients", threshold) - http.Error(w, "429 Too Many Requests", 429) + http.Error(w, "429 Too Many Requests", http.StatusTooManyRequests) return } @@ -334,24 +332,6 @@ func throttled(threshold int64, h http.Handler) http.Handler { }) } -// sanitizeRequest returns a sanitized string for the request which may be -// safely logged. It is intended to strip private keys, passphrases, and any -// other secrets from request parameters before they may be saved to a log file. -func sanitizeRequest(r *btcjson.Request) string { - // These are considered unsafe to log, so sanitize parameters. - switch r.Method { - case "encryptwallet", "importprivkey", "importwallet", - "signrawtransaction", "walletpassphrase", - "walletpassphrasechange": - - return fmt.Sprintf(`{"id":%v,"method":"%s","params":SANITIZED %d parameters}`, - r.ID, r.Method, len(r.Params)) - } - - return fmt.Sprintf(`{"id":%v,"method":"%s","params":%v}`, r.ID, - r.Method, r.Params) -} - // idPointer returns a pointer to the passed ID, or nil if the interface is nil. // Interface pointers are usually a red flag of doing something incorrectly, // but this is only implemented here to work around an oddity with btcjson, @@ -473,7 +453,7 @@ out: break out } s.requestProcessShutdown() - break + break out default: req := req // Copy for the closure diff --git a/rpc/rpcserver/log.go b/rpc/rpcserver/log.go index d25317a..36e3566 100644 --- a/rpc/rpcserver/log.go +++ b/rpc/rpcserver/log.go @@ -25,7 +25,7 @@ import ( // UseLogger sets the logger to use for the gRPC server. func UseLogger(l btclog.Logger) { - grpclog.SetLogger(logger{l}) + grpclog.SetLogger(logger{l}) // nolint:staticcheck } // logger uses a btclog.Logger to implement the grpclog.Logger interface. diff --git a/rpc/rpcserver/server.go b/rpc/rpcserver/server.go index fa3bf58..e9efe59 100644 --- a/rpc/rpcserver/server.go +++ b/rpc/rpcserver/server.go @@ -24,6 +24,7 @@ import ( "golang.org/x/net/context" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/rpcclient" @@ -49,7 +50,7 @@ const ( semverPatch = 1 ) -// translateError creates a new gRPC error with an appropiate error code for +// translateError creates a new gRPC error with an appropriate error code for // recognized errors. // // This function is by no means complete and should be expanded based on other @@ -57,7 +58,7 @@ const ( // should return this result instead. func translateError(err error) error { code := errorCode(err) - return grpc.Errorf(code, "%s", err.Error()) + return status.Errorf(code, "%s", err.Error()) } func errorCode(err error) codes.Code { @@ -202,7 +203,7 @@ func (s *walletServer) NextAccount(ctx context.Context, req *pb.NextAccountReque defer zero.Bytes(req.Passphrase) if req.AccountName == "" { - return nil, grpc.Errorf(codes.InvalidArgument, "account name may not be empty") + return nil, status.Errorf(codes.InvalidArgument, "account name may not be empty") } lock := make(chan time.Time, 1) @@ -235,7 +236,7 @@ func (s *walletServer) NextAddress(ctx context.Context, req *pb.NextAddressReque case pb.NextAddressRequest_BIP0044_INTERNAL: addr, err = s.wallet.NewChangeAddress(req.Account, waddrmgr.KeyScopeBIP0044) default: - return nil, grpc.Errorf(codes.InvalidArgument, "kind=%v", req.Kind) + return nil, status.Errorf(codes.InvalidArgument, "kind=%v", req.Kind) } if err != nil { return nil, translateError(err) @@ -251,7 +252,7 @@ func (s *walletServer) ImportPrivateKey(ctx context.Context, req *pb.ImportPriva wif, err := btcutil.DecodeWIF(req.PrivateKeyWif) if err != nil { - return nil, grpc.Errorf(codes.InvalidArgument, + return nil, status.Errorf(codes.InvalidArgument, "Invalid WIF-encoded private key: %v", err) } @@ -267,7 +268,7 @@ func (s *walletServer) ImportPrivateKey(ctx context.Context, req *pb.ImportPriva // At the moment, only the special-cased import account can be used to // import keys. if req.Account != waddrmgr.ImportedAddrAccount { - return nil, grpc.Errorf(codes.InvalidArgument, + return nil, status.Errorf(codes.InvalidArgument, "Only the imported account accepts private key imports") } @@ -299,24 +300,6 @@ func (s *walletServer) Balance(ctx context.Context, req *pb.BalanceRequest) ( return resp, nil } -// confirmed checks whether a transaction at height txHeight has met minconf -// confirmations for a blockchain at height curHeight. -func confirmed(minconf, txHeight, curHeight int32) bool { - return confirms(txHeight, curHeight) >= minconf -} - -// confirms returns the number of confirmations for a transaction in a block at -// height txHeight (or -1 for an unconfirmed tx) given the chain height -// curHeight. -func confirms(txHeight, curHeight int32) int32 { - switch { - case txHeight == -1, txHeight > curHeight: - return 0 - default: - return curHeight - txHeight + 1 - } -} - func (s *walletServer) FundTransaction(ctx context.Context, req *pb.FundTransactionRequest) ( *pb.FundTransactionResponse, error) { @@ -383,26 +366,26 @@ func (s *walletServer) GetTransactions(ctx context.Context, req *pb.GetTransacti resp *pb.GetTransactionsResponse, err error) { var startBlock, endBlock *wallet.BlockIdentifier - if req.StartingBlockHash != nil && req.StartingBlockHeight != 0 { + if req.StartingBlockHash != nil && req.StartingBlockHeight != 0 { // nolint:gocritic return nil, errors.New( "starting block hash and height may not be specified simultaneously") } else if req.StartingBlockHash != nil { startBlockHash, err := chainhash.NewHash(req.StartingBlockHash) if err != nil { - return nil, grpc.Errorf(codes.InvalidArgument, "%s", err.Error()) + return nil, status.Errorf(codes.InvalidArgument, "%s", err.Error()) } startBlock = wallet.NewBlockIdentifierFromHash(startBlockHash) } else if req.StartingBlockHeight != 0 { startBlock = wallet.NewBlockIdentifierFromHeight(req.StartingBlockHeight) } - if req.EndingBlockHash != nil && req.EndingBlockHeight != 0 { - return nil, grpc.Errorf(codes.InvalidArgument, + if req.EndingBlockHash != nil && req.EndingBlockHeight != 0 { // nolint:gocritic + return nil, status.Errorf(codes.InvalidArgument, "ending block hash and height may not be specified simultaneously") } else if req.EndingBlockHash != nil { endBlockHash, err := chainhash.NewHash(req.EndingBlockHash) if err != nil { - return nil, grpc.Errorf(codes.InvalidArgument, "%s", err.Error()) + return nil, status.Errorf(codes.InvalidArgument, "%s", err.Error()) } endBlock = wallet.NewBlockIdentifierFromHash(endBlockHash) } else if req.EndingBlockHeight != 0 { @@ -412,13 +395,13 @@ func (s *walletServer) GetTransactions(ctx context.Context, req *pb.GetTransacti var minRecentTxs int if req.MinimumRecentTransactions != 0 { if endBlock != nil { - return nil, grpc.Errorf(codes.InvalidArgument, + return nil, status.Errorf(codes.InvalidArgument, "ending block and minimum number of recent transactions "+ "may not be specified simultaneously") } minRecentTxs = int(req.MinimumRecentTransactions) if minRecentTxs < 0 { - return nil, grpc.Errorf(codes.InvalidArgument, + return nil, status.Errorf(codes.InvalidArgument, "minimum number of recent transactions may not be negative") } } @@ -447,7 +430,7 @@ func (s *walletServer) ChangePassphrase(ctx context.Context, req *pb.ChangePassp case pb.ChangePassphraseRequest_PUBLIC: err = s.wallet.ChangePublicPassphrase(req.OldPassphrase, req.NewPassphrase) default: - return nil, grpc.Errorf(codes.InvalidArgument, "Unknown key type (%d)", req.Key) + return nil, status.Errorf(codes.InvalidArgument, "Unknown key type (%d)", req.Key) } if err != nil { return nil, translateError(err) @@ -465,7 +448,7 @@ func (s *walletServer) SignTransaction(ctx context.Context, req *pb.SignTransact var tx wire.MsgTx err := tx.Deserialize(bytes.NewReader(req.SerializedTransaction)) if err != nil { - return nil, grpc.Errorf(codes.InvalidArgument, + return nil, status.Errorf(codes.InvalidArgument, "Bytes do not represent a valid raw transaction: %v", err) } @@ -515,7 +498,7 @@ func (s *walletServer) PublishTransaction(ctx context.Context, req *pb.PublishTr var msgTx wire.MsgTx err := msgTx.Deserialize(bytes.NewReader(req.SignedTransaction)) if err != nil { - return nil, grpc.Errorf(codes.InvalidArgument, + return nil, status.Errorf(codes.InvalidArgument, "Bytes do not represent a valid raw transaction: %v", err) } @@ -591,18 +574,6 @@ func marshalHashes(v []*chainhash.Hash) [][]byte { return hashes } -func marshalAccountBalances(v []wallet.AccountBalance) []*pb.AccountBalance { - balances := make([]*pb.AccountBalance, len(v)) - for i := range v { - balance := &v[i] - balances[i] = &pb.AccountBalance{ - Account: balance.Account, - TotalBalance: int64(balance.TotalBalance), - } - } - return balances -} - func (s *walletServer) TransactionNotifications(req *pb.TransactionNotificationsRequest, svr pb.WalletService_TransactionNotificationsServer) error { @@ -634,7 +605,7 @@ func (s *walletServer) SpentnessNotifications(req *pb.SpentnessNotificationsRequ svr pb.WalletService_SpentnessNotificationsServer) error { if req.NoNotifyUnspent && req.NoNotifySpent { - return grpc.Errorf(codes.InvalidArgument, + return status.Errorf(codes.InvalidArgument, "no_notify_unspent and no_notify_spent may not both be true") } @@ -776,7 +747,7 @@ func (s *loaderServer) CloseWallet(ctx context.Context, req *pb.CloseWalletReque err := s.loader.UnloadWallet() if err == wallet.ErrNotLoaded { - return nil, grpc.Errorf(codes.FailedPrecondition, "wallet is not loaded") + return nil, status.Errorf(codes.FailedPrecondition, "wallet is not loaded") } if err != nil { return nil, translateError(err) @@ -785,8 +756,9 @@ func (s *loaderServer) CloseWallet(ctx context.Context, req *pb.CloseWalletReque return &pb.CloseWalletResponse{}, nil } -func (s *loaderServer) StartConsensusRpc(ctx context.Context, req *pb.StartConsensusRpcRequest) ( - *pb.StartConsensusRpcResponse, error) { +func (s *loaderServer) StartConsensusRpc(ctx context.Context, // nolint:golint + req *pb.StartConsensusRpcRequest) (*pb.StartConsensusRpcResponse, + error) { defer zero.Bytes(req.Password) @@ -794,20 +766,20 @@ func (s *loaderServer) StartConsensusRpc(ctx context.Context, req *pb.StartConse s.mu.Lock() if s.rpcClient != nil { - return nil, grpc.Errorf(codes.FailedPrecondition, "RPC client already created") + return nil, status.Errorf(codes.FailedPrecondition, "RPC client already created") } networkAddress, err := cfgutil.NormalizeAddress(req.NetworkAddress, s.activeNet.RPCClientPort) if err != nil { - return nil, grpc.Errorf(codes.InvalidArgument, + return nil, status.Errorf(codes.InvalidArgument, "Network address is ill-formed: %v", err) } // Error if the wallet is already syncing with the network. wallet, walletLoaded := s.loader.LoadedWallet() if walletLoaded && wallet.SynchronizingToNetwork() { - return nil, grpc.Errorf(codes.FailedPrecondition, + return nil, status.Errorf(codes.FailedPrecondition, "wallet is loaded and already synchronizing") } @@ -820,10 +792,10 @@ func (s *loaderServer) StartConsensusRpc(ctx context.Context, req *pb.StartConse err = rpcClient.Start() if err != nil { if err == rpcclient.ErrInvalidAuth { - return nil, grpc.Errorf(codes.InvalidArgument, + return nil, status.Errorf(codes.InvalidArgument, "Invalid RPC credentials: %v", err) } - return nil, grpc.Errorf(codes.NotFound, + return nil, status.Errorf(codes.NotFound, "Connection to RPC server failed: %v", err) } diff --git a/snacl/snacl.go b/snacl/snacl.go index 1f09531..fca7d28 100644 --- a/snacl/snacl.go +++ b/snacl/snacl.go @@ -222,7 +222,7 @@ func (sk *SecretKey) Decrypt(in []byte) ([]byte, error) { } // NewSecretKey returns a SecretKey structure based on the passed parameters. -func NewSecretKey(password *[]byte, N, r, p int) (*SecretKey, error) { +func NewSecretKey(password *[]byte, N, r, p int) (*SecretKey, error) { // nolint:gocritic sk := SecretKey{ Key: (*CryptoKey)(&[KeySize]byte{}), } diff --git a/waddrmgr/address.go b/waddrmgr/address.go index 3763a1c..45df8e0 100644 --- a/waddrmgr/address.go +++ b/waddrmgr/address.go @@ -130,7 +130,6 @@ type managedAddress struct { imported bool internal bool compressed bool - used bool addrType AddressType pubKey *btcec.PublicKey privKeyEncrypted []byte // nil if part of watch-only account @@ -367,7 +366,7 @@ func newManagedAddressWithoutPrivKey(m *ScopedKeyManager, case NestedWitnessPubKey: // For this address type we'l generate an address which is // backwards compatible to Bitcoin nodes running 0.6.0 onwards, but - // allows us to take advantage of segwit's scripting improvments, + // allows us to take advantage of segwit's scripting improvements, // and malleability fixes. // First, we'll generate a normal p2wkh address from the pubkey hash. @@ -511,7 +510,6 @@ type scriptAddress struct { scriptEncrypted []byte scriptCT []byte scriptMutex sync.Mutex - used bool } // Enforce scriptAddress satisfies the ManagedScriptAddress interface. diff --git a/waddrmgr/db.go b/waddrmgr/db.go index 1d8f6cf..9ad1a59 100644 --- a/waddrmgr/db.go +++ b/waddrmgr/db.go @@ -61,7 +61,7 @@ type syncStatus uint8 // of supporting sync status on a per-address basis. const ( ssNone syncStatus = 0 // not iota as they need to be stable for db - ssPartial syncStatus = 1 + ssPartial syncStatus = 1 // nolint:varcheck,deadcode,unused ssFull syncStatus = 2 ) @@ -294,14 +294,6 @@ func uint32ToBytes(number uint32) []byte { return buf } -// uint64ToBytes converts a 64 bit unsigned integer into a 8-byte slice in -// little-endian order: 1 -> [1 0 0 0 0 0 0 0]. -func uint64ToBytes(number uint64) []byte { - buf := make([]byte, 8) - binary.LittleEndian.PutUint64(buf, number) - return buf -} - // stringToBytes converts a string into a variable length byte slice in // little-endian order: "abc" -> [3 0 0 0 61 62 63] func stringToBytes(s string) []byte { @@ -330,15 +322,6 @@ func scopeToBytes(scope *KeyScope) [scopeKeySize]byte { return scopeBytes } -// scopeFromBytes decodes a serializes manager scope into its concrete manager -// scope struct. -func scopeFromBytes(scopeBytes []byte) KeyScope { - return KeyScope{ - Purpose: binary.LittleEndian.Uint32(scopeBytes[:]), - Coin: binary.LittleEndian.Uint32(scopeBytes[4:]), - } -} - // scopeSchemaToBytes encodes the passed scope schema as a set of bytes // suitable for storage within the database. func scopeSchemaToBytes(schema *ScopeAddrSchema) []byte { @@ -380,22 +363,6 @@ func fetchScopeAddrSchema(ns walletdb.ReadBucket, return scopeSchemaFromBytes(schemaBytes), nil } -// putScopeAddrSchema attempts to store the passed addr scehma for the given -// manager scope. -func putScopeAddrTypes(ns walletdb.ReadWriteBucket, scope *KeyScope, - schema *ScopeAddrSchema) error { - - scopeSchemaBucket := ns.NestedReadWriteBucket(scopeSchemaBucketName) - if scopeSchemaBucket == nil { - str := fmt.Sprintf("unable to find scope schema bucket") - return managerError(ErrScopeNotFound, str, nil) - } - - scopeKey := scopeToBytes(scope) - schemaBytes := scopeSchemaToBytes(schema) - return scopeSchemaBucket.Put(scopeKey[:], schemaBytes) -} - func fetchReadScopeBucket(ns walletdb.ReadBucket, scope *KeyScope) (walletdb.ReadBucket, error) { rootScopeBucket := ns.NestedReadBucket(scopeBucketName) @@ -590,7 +557,7 @@ func putMasterHDKeys(ns walletdb.ReadWriteBucket, masterHDPrivEnc, masterHDPubEn // fetchMasterHDKeys attempts to fetch both the master HD private and public // keys from the database. If this is a watch only wallet, then it's possible // that the master private key isn't stored. -func fetchMasterHDKeys(ns walletdb.ReadBucket) ([]byte, []byte, error) { +func fetchMasterHDKeys(ns walletdb.ReadBucket) ([]byte, []byte) { bucket := ns.NestedReadBucket(mainBucketName) var masterHDPrivEnc, masterHDPubEnc []byte @@ -601,16 +568,16 @@ func fetchMasterHDKeys(ns walletdb.ReadBucket) ([]byte, []byte, error) { key := bucket.Get(masterHDPrivName) if key != nil { masterHDPrivEnc = make([]byte, len(key)) - copy(masterHDPrivEnc[:], key) + copy(masterHDPrivEnc, key) } key = bucket.Get(masterHDPubName) if key != nil { masterHDPubEnc = make([]byte, len(key)) - copy(masterHDPubEnc[:], key) + copy(masterHDPubEnc, key) } - return masterHDPrivEnc, masterHDPubEnc, nil + return masterHDPrivEnc, masterHDPubEnc } // fetchCryptoKeys loads the encrypted crypto keys which are in turn used to @@ -993,7 +960,7 @@ func forEachKeyScope(ns walletdb.ReadBucket, fn func(KeyScope) error) error { } scope := KeyScope{ - Purpose: binary.LittleEndian.Uint32(k[:]), + Purpose: binary.LittleEndian.Uint32(k), Coin: binary.LittleEndian.Uint32(k[4:]), } @@ -1545,7 +1512,7 @@ func fetchAddressByHash(ns walletdb.ReadBucket, scope *KeyScope, bucket := scopedBucket.NestedReadBucket(addrBucketName) - serializedRow := bucket.Get(addrHash[:]) + serializedRow := bucket.Get(addrHash) if serializedRow == nil { str := "address not found" return nil, managerError(ErrAddressNotFound, str, nil) @@ -2349,7 +2316,7 @@ func fetchBirthdayBlockVerification(ns walletdb.ReadBucket) bool { } // Otherwise, we'll determine if it's verified by the value stored. - verified := binary.BigEndian.Uint16(verifiedValue[:]) + verified := binary.BigEndian.Uint16(verifiedValue) return verified != 0 } @@ -2483,6 +2450,8 @@ func createManagerNS(ns walletdb.ReadWriteBucket, // Next, we'll create the namespace for each of the relevant default // manager scopes. for scope, scopeSchema := range defaultScopes { + scope, scopeSchema := scope, scopeSchema + // Before we create the entire namespace of this scope, we'll // update the schema mapping to note what types of addresses it // prefers. diff --git a/waddrmgr/log.go b/waddrmgr/log.go index 44fa6e0..1f41f04 100644 --- a/waddrmgr/log.go +++ b/waddrmgr/log.go @@ -24,20 +24,3 @@ func DisableLog() { func UseLogger(logger btclog.Logger) { log = logger } - -// LogClosure is a closure that can be printed with %v to be used to -// generate expensive-to-create data for a detailed log level and avoid doing -// the work if the data isn't printed. -type logClosure func() string - -// String invokes the log closure and returns the results string. -func (c logClosure) String() string { - return c() -} - -// newLogClosure returns a new closure over the passed function which allows -// it to be used as a parameter in a logging function that is only invoked when -// the logging level is such that the message will actually be logged. -func newLogClosure(c func() string) logClosure { - return logClosure(c) -} diff --git a/waddrmgr/manager.go b/waddrmgr/manager.go index 72a7984..cf7b9aa 100644 --- a/waddrmgr/manager.go +++ b/waddrmgr/manager.go @@ -474,7 +474,6 @@ func (m *Manager) Close() { m.masterKeyPub.Zero() m.closed = true - return } // NewScopedKeyManager creates a new scoped key manager from the root manager. A @@ -509,10 +508,7 @@ func (m *Manager) NewScopedKeyManager(ns walletdb.ReadWriteBucket, // Note that the path to the coin type is requires hardened // derivation, therefore this can only be done if the wallet's // root key hasn't been neutered. - masterRootPrivEnc, _, err := fetchMasterHDKeys(ns) - if err != nil { - return nil, err - } + masterRootPrivEnc, _ := fetchMasterHDKeys(ns) // If the master root private key isn't found within the // database, but we need to bail here as we can't create the @@ -636,8 +632,7 @@ func (m *Manager) ScopesForExternalAddrType(addrType AddressType) []KeyScope { m.mtx.RLock() defer m.mtx.RUnlock() - scopes, _ := m.externalAddrSchemas[addrType] - return scopes + return m.externalAddrSchemas[addrType] } // ScopesForInternalAddrTypes returns the set of key scopes that are able to @@ -646,8 +641,7 @@ func (m *Manager) ScopesForInternalAddrTypes(addrType AddressType) []KeyScope { m.mtx.RLock() defer m.mtx.RUnlock() - scopes, _ := m.internalAddrSchemas[addrType] - return scopes + return m.internalAddrSchemas[addrType] } // NeuterRootKey is a special method that should be used once a caller is @@ -658,10 +652,7 @@ func (m *Manager) NeuterRootKey(ns walletdb.ReadWriteBucket) error { defer m.mtx.Unlock() // First, we'll fetch the current master HD keys from the database. - masterRootPrivEnc, _, err := fetchMasterHDKeys(ns) - if err != nil { - return err - } + masterRootPrivEnc, _ := fetchMasterHDKeys(ns) // If the root master private key is already nil, then we'll return a // nil error here as the root key has already been permanently @@ -984,8 +975,8 @@ func (m *Manager) ChangePassphrase(ns walletdb.ReadWriteBucket, oldPassphrase, // Now that the db has been successfully updated, clear the old // key and set the new one. - copy(m.cryptoKeyPrivEncrypted[:], encPriv) - copy(m.cryptoKeyScriptEncrypted[:], encScript) + copy(m.cryptoKeyPrivEncrypted, encPriv) + copy(m.cryptoKeyScriptEncrypted, encScript) m.masterKeyPriv.Zero() // Clear the old key. m.masterKeyPriv = newMasterKey m.privPassphraseSalt = passphraseSalt @@ -1421,7 +1412,7 @@ func deriveCoinTypeKey(masterNode *hdkeychain.ExtendedKey, // The branch is 0 for external addresses and 1 for internal addresses. // Derive the purpose key as a child of the master node. - purpose, err := masterNode.DeriveNonStandard( + purpose, err := masterNode.DeriveNonStandard( // nolint:staticcheck scope.Purpose + hdkeychain.HardenedKeyStart, ) if err != nil { @@ -1429,7 +1420,7 @@ func deriveCoinTypeKey(masterNode *hdkeychain.ExtendedKey, } // Derive the coin type key as a child of the purpose key. - coinTypeKey, err := purpose.DeriveNonStandard( + coinTypeKey, err := purpose.DeriveNonStandard( // nolint:staticcheck scope.Coin + hdkeychain.HardenedKeyStart, ) if err != nil { @@ -1454,7 +1445,7 @@ func deriveAccountKey(coinTypeKey *hdkeychain.ExtendedKey, } // Derive the account key as a child of the coin type key. - return coinTypeKey.DeriveNonStandard( + return coinTypeKey.DeriveNonStandard( // nolint:staticcheck account + hdkeychain.HardenedKeyStart, ) } @@ -1471,12 +1462,12 @@ func deriveAccountKey(coinTypeKey *hdkeychain.ExtendedKey, // The branch is 0 for external addresses and 1 for internal addresses. func checkBranchKeys(acctKey *hdkeychain.ExtendedKey) error { // Derive the external branch as the first child of the account key. - if _, err := acctKey.DeriveNonStandard(ExternalBranch); err != nil { + if _, err := acctKey.DeriveNonStandard(ExternalBranch); err != nil { // nolint:staticcheck return err } // Derive the internal branch as the second child of the account key. - _, err := acctKey.DeriveNonStandard(InternalBranch) + _, err := acctKey.DeriveNonStandard(InternalBranch) // nolint:staticcheck return err } diff --git a/waddrmgr/manager_test.go b/waddrmgr/manager_test.go index d574302..557bae4 100644 --- a/waddrmgr/manager_test.go +++ b/waddrmgr/manager_test.go @@ -43,18 +43,6 @@ func (c *failingCryptoKey) Decrypt(in []byte) ([]byte, error) { return nil, errors.New("failed to decrypt") } -// newHash converts the passed big-endian hex string into a chainhash.Hash. -// It only differs from the one available in wire in that it panics on an -// error since it will only (and must only) be called with hard-coded, and -// therefore known good, hashes. -func newHash(hexStr string) *chainhash.Hash { - hash, err := chainhash.NewHashFromStr(hexStr) - if err != nil { - panic(err) - } - return hash -} - // failingSecretKeyGen is a SecretKeyGenerator that always returns // snacl.ErrDecryptFailed. func failingSecretKeyGen(passphrase *[]byte, @@ -97,7 +85,6 @@ type expectedAddr struct { addressHash []byte internal bool compressed bool - used bool imported bool pubKey []byte privKey []byte @@ -628,11 +615,7 @@ func testInternalAddresses(tc *testContext) bool { return false } tc.unlocked = false - if !testResults() { - return false - } - - return true + return testResults() } // testLocking tests the basic locking semantics of the address manager work @@ -788,6 +771,8 @@ func testImportPrivateKey(tc *testContext) bool { prefix := testNamePrefix(tc) + " testImportPrivateKey" if tc.create { for i, test := range tests { + test := test + test.expected.privKeyWIF = test.in wif, err := btcutil.DecodeWIF(test.in) if err != nil { @@ -799,7 +784,9 @@ func testImportPrivateKey(tc *testContext) bool { err = walletdb.Update(tc.db, func(tx walletdb.ReadWriteTx) error { ns := tx.ReadWriteBucket(waddrmgrNamespaceKey) var err error - addr, err = tc.manager.ImportPrivateKey(ns, wif, &test.blockstamp) + addr, err = tc.manager.ImportPrivateKey( + ns, wif, &test.blockstamp, + ) return err }) if err != nil { @@ -883,11 +870,7 @@ func testImportPrivateKey(tc *testContext) bool { return false } tc.unlocked = false - if !testResults() { - return false - } - - return true + return testResults() } // testImportScript tests that importing scripts works properly. It ensures @@ -961,6 +944,7 @@ func testImportScript(tc *testContext) bool { prefix := testNamePrefix(tc) if tc.create { for i, test := range tests { + test := test test.expected.script = test.in prefix := fmt.Sprintf("%s ImportScript #%d (%s)", prefix, i, test.name) @@ -1051,11 +1035,7 @@ func testImportScript(tc *testContext) bool { return false } tc.unlocked = false - if !testResults() { - return false - } - - return true + return testResults() } // testMarkUsed ensures used addresses are flagged as such. @@ -1080,6 +1060,8 @@ func testMarkUsed(tc *testContext, doScript bool) bool { prefix := fmt.Sprintf("(%s) MarkUsed", tc.caseName) chainParams := tc.manager.ChainParams() for i, test := range tests { + i, test := i, test + if !doScript && test.typ == addrScriptHash { continue } @@ -1380,10 +1362,7 @@ func testNewAccount(tc *testContext) bool { return err }) wantErrCode = ErrInvalidAccount - if !checkManagerError(tc.t, testName, err, wantErrCode) { - return false - } - return true + return checkManagerError(tc.t, testName, err, wantErrCode) } // testLookupAccount tests the basic account lookup func of the address manager @@ -1406,7 +1385,10 @@ func testLookupAccount(tc *testContext) bool { func testLookupExpectedAccount(tc *testContext, expectedAccounts map[string]uint32, expectedLastAccount uint32) bool { + for acctName, expectedAccount := range expectedAccounts { + acctName := acctName + var account uint32 err := walletdb.View(tc.db, func(tx walletdb.ReadTx) error { ns := tx.ReadBucket(waddrmgrNamespaceKey) @@ -1445,6 +1427,10 @@ func testLookupExpectedAccount(tc *testContext, expectedAccounts map[string]uint lastAccount, err = tc.manager.LastAccount(ns) return err }) + if err != nil { + tc.t.Errorf("LookupAccount: unexpected error: %v", err) + return false + } if lastAccount != expectedLastAccount { tc.t.Errorf("LookupAccount "+ "account mismatch -- got %d, "+ @@ -1537,10 +1523,7 @@ func testRenameAccount(tc *testContext) bool { return err }) wantErrCode = ErrAccountNotFound - if !checkManagerError(tc.t, testName, err, wantErrCode) { - return false - } - return true + return checkManagerError(tc.t, testName, err, wantErrCode) } // testForEachAccount tests the retrieve all accounts func of the address @@ -2727,7 +2710,7 @@ func TestNewRawAccountHybrid(t *testing.T) { testNewRawAccount(t, mgr, db, accountNum, scopedMgr) } -func testNewRawAccount(t *testing.T, mgr *Manager, db walletdb.DB, +func testNewRawAccount(t *testing.T, _ *Manager, db walletdb.DB, accountNum uint32, scopedMgr *ScopedKeyManager) { // With the account created, we should be able to derive new addresses // from the account. diff --git a/waddrmgr/migrations.go b/waddrmgr/migrations.go index 95e1f98..77e0080 100644 --- a/waddrmgr/migrations.go +++ b/waddrmgr/migrations.go @@ -93,7 +93,7 @@ func (m *MigrationManager) SetVersion(ns walletdb.ReadWriteBucket, if ns == nil { ns = m.ns } - return putManagerVersion(m.ns, version) + return putManagerVersion(ns, version) } // Versions returns all of the available database versions of the service. diff --git a/waddrmgr/migrations_test.go b/waddrmgr/migrations_test.go index 92696c0..c7aaeba 100644 --- a/waddrmgr/migrations_test.go +++ b/waddrmgr/migrations_test.go @@ -327,6 +327,7 @@ func TestMigrationStoreMaxReorgDepth(t *testing.T) { } for _, testCase := range testCases { + testCase := testCase success := t.Run(testCase.name, func(t *testing.T) { // We'll start the test by creating the number of blocks // we'll add to the chain. We start from height 1 as the @@ -387,13 +388,13 @@ func TestMigrationStoreMaxReorgDepth(t *testing.T) { return err } expectedBlock := blocks[len(blocks)-1] - if block.Height != block.Height { + if expectedBlock.Height != block.Height { return fmt.Errorf("expected synced to "+ "block height %v, got %v", expectedBlock.Height, block.Height) } - if block.Hash != block.Hash { + if expectedBlock.Hash != block.Hash { return fmt.Errorf("expected synced to "+ "block hash %v, got %v", expectedBlock.Hash, diff --git a/waddrmgr/scoped_manager.go b/waddrmgr/scoped_manager.go index b91ccef..70fe48b 100644 --- a/waddrmgr/scoped_manager.go +++ b/waddrmgr/scoped_manager.go @@ -255,7 +255,6 @@ func (s *ScopedKeyManager) Close() { // Attempt to clear sensitive public key material from memory too. s.zeroSensitivePublicData() - return } // keyToManaged returns a new managed address for the provided derived key and @@ -318,15 +317,17 @@ func (s *ScopedKeyManager) deriveKey(acctInfo *accountInfo, branch, } // Derive and return the key. - branchKey, err := acctKey.DeriveNonStandard(branch) + branchKey, err := acctKey.DeriveNonStandard(branch) // nolint:staticcheck if err != nil { str := fmt.Sprintf("failed to derive extended key branch %d", branch) return nil, managerError(ErrKeyChain, str, err) } - addressKey, err := branchKey.DeriveNonStandard(index) - branchKey.Zero() // Zero branch key after it's used. + addressKey, err := branchKey.DeriveNonStandard(index) // nolint:staticcheck + + // Zero branch key after it's used. + branchKey.Zero() if err != nil { str := fmt.Sprintf("failed to derive child extended key -- "+ "branch %d, child %d", @@ -430,7 +431,6 @@ func (s *ScopedKeyManager) loadAccountInfo(ns walletdb.ReadBucket, return nil, managerError(ErrCrypto, str, err) } - watchOnly = true hasPrivateKey = false default: @@ -879,7 +879,7 @@ func (s *ScopedKeyManager) nextAddresses(ns walletdb.ReadWriteBucket, } // Derive the appropriate branch key and ensure it is zeroed when done. - branchKey, err := acctKey.DeriveNonStandard(branchNum) + branchKey, err := acctKey.DeriveNonStandard(branchNum) // nolint:staticcheck if err != nil { str := fmt.Sprintf("failed to derive extended key branch %d", branchNum) @@ -896,7 +896,7 @@ func (s *ScopedKeyManager) nextAddresses(ns walletdb.ReadWriteBucket, var nextKey *hdkeychain.ExtendedKey for { // Derive the next child in the external chain branch. - key, err := branchKey.DeriveNonStandard(nextIndex) + key, err := branchKey.DeriveNonStandard(nextIndex) // nolint:staticcheck if err != nil { // When this particular child is invalid, skip to the // next index. @@ -1081,7 +1081,7 @@ func (s *ScopedKeyManager) extendAddresses(ns walletdb.ReadWriteBucket, } // Derive the appropriate branch key and ensure it is zeroed when done. - branchKey, err := acctKey.DeriveNonStandard(branchNum) + branchKey, err := acctKey.DeriveNonStandard(branchNum) // nolint:staticcheck if err != nil { str := fmt.Sprintf("failed to derive extended key branch %d", branchNum) @@ -1100,7 +1100,7 @@ func (s *ScopedKeyManager) extendAddresses(ns walletdb.ReadWriteBucket, var nextKey *hdkeychain.ExtendedKey for { // Derive the next child in the external chain branch. - key, err := branchKey.DeriveNonStandard(nextIndex) + key, err := branchKey.DeriveNonStandard(nextIndex) // nolint:staticcheck if err != nil { // When this particular child is invalid, skip to the // next index. diff --git a/wallet/chainntfns.go b/wallet/chainntfns.go index 32f2acd..990d8bc 100644 --- a/wallet/chainntfns.go +++ b/wallet/chainntfns.go @@ -36,7 +36,7 @@ func (w *Wallet) handleChainNotifications() { catchUpHashes := func(w *Wallet, client chain.Interface, height int32) error { - // TODO(aakselrod): There's a race conditon here, which + // TODO(aakselrod): There's a race condition here, which // happens when a reorg occurs between the // rescanProgress notification and the last GetBlockHash // call. The solution when using btcd is to make btcd @@ -107,14 +107,14 @@ func (w *Wallet) handleChainNotifications() { chainClient, birthdayStore, ) if err != nil && !waddrmgr.IsError(err, waddrmgr.ErrBirthdayBlockNotSet) { - panic(fmt.Errorf("Unable to sanity "+ + panic(fmt.Errorf("unable to sanity "+ "check wallet birthday block: %v", err)) } err = w.syncWithChain(birthdayBlock) if err != nil && !w.ShuttingDown() { - panic(fmt.Errorf("Unable to synchronize "+ + panic(fmt.Errorf("unable to synchronize "+ "wallet to chain: %v", err)) } case chain.BlockConnected: diff --git a/wallet/createtx.go b/wallet/createtx.go index 0ebc1a1..358e650 100644 --- a/wallet/createtx.go +++ b/wallet/createtx.go @@ -118,7 +118,7 @@ func (w *Wallet) txToOutputs(outputs []*wire.TxOut, account uint32, if err != nil { return nil, err } - defer dbtx.Rollback() + defer func() { _ = dbtx.Rollback() }() addrmgrNs, changeSource := w.addrMgrWithChangeSource(dbtx, account) @@ -206,7 +206,7 @@ func (w *Wallet) findEligibleOutputs(dbtx walletdb.ReadTx, account uint32, minco // should be handled by the call to UnspentOutputs (or similar). // Because one of these filters requires matching the output script to // the desired account, this change depends on making wtxmgr a waddrmgr - // dependancy and requesting unspent outputs for a single account. + // dependency and requesting unspent outputs for a single account. eligible := make([]wtxmgr.Credit, 0, len(unspent)) for i := range unspent { output := &unspent[i] diff --git a/wallet/import.go b/wallet/import.go index ce867ed..0958434 100644 --- a/wallet/import.go +++ b/wallet/import.go @@ -350,7 +350,7 @@ func (w *Wallet) ImportPrivateKey(scope waddrmgr.KeyScope, wif *btcutil.WIF, } else { err := w.chainClient.NotifyReceived([]btcutil.Address{addr}) if err != nil { - return "", fmt.Errorf("Failed to subscribe for address ntfns for "+ + return "", fmt.Errorf("failed to subscribe for address ntfns for "+ "address %s: %s", addr.EncodeAddress(), err) } } diff --git a/wallet/import_test.go b/wallet/import_test.go index 517ff97..2d75cca 100644 --- a/wallet/import_test.go +++ b/wallet/import_test.go @@ -66,7 +66,6 @@ type testCase struct { masterPriv string accountIndex uint32 addrType waddrmgr.AddressType - addrSchemaOverride *waddrmgr.ScopeAddrSchema expectedScope waddrmgr.KeyScope expectedAddr string expectedChangeAddr string @@ -100,7 +99,6 @@ var ( "FD2KeY6G9", accountIndex: 9, addrType: waddrmgr.NestedWitnessPubKey, - addrSchemaOverride: &waddrmgr.KeyScopeBIP0049AddrSchema, expectedScope: waddrmgr.KeyScopeBIP0049Plus, expectedAddr: "2NBCJ9WzGXZqpLpXGq3Hacybj3c4eHRcqgh", expectedChangeAddr: "2N3bankFu6F3ZNU41iVJQqyS9MXqp9dvn1M", diff --git a/wallet/log.go b/wallet/log.go index 1756ea8..de39fe1 100644 --- a/wallet/log.go +++ b/wallet/log.go @@ -38,23 +38,6 @@ func UseLogger(logger btclog.Logger) { wtxmgr.UseLogger(logger) } -// LogClosure is a closure that can be printed with %v to be used to -// generate expensive-to-create data for a detailed log level and avoid doing -// the work if the data isn't printed. -type logClosure func() string - -// String invokes the log closure and returns the results string. -func (c logClosure) String() string { - return c() -} - -// newLogClosure returns a new closure over the passed function which allows -// it to be used as a parameter in a logging function that is only invoked when -// the logging level is such that the message will actually be logged. -func newLogClosure(c func() string) logClosure { - return logClosure(c) -} - // pickNoun returns the singular or plural form of a noun depending // on the count n. func pickNoun(n int, singular, plural string) string { diff --git a/wallet/multisig.go b/wallet/multisig.go index a1f9baa..7f66181 100644 --- a/wallet/multisig.go +++ b/wallet/multisig.go @@ -27,7 +27,7 @@ func (w *Wallet) MakeMultiSigScript(addrs []btcutil.Address, nRequired int) ([]b var addrmgrNs walletdb.ReadBucket defer func() { if dbtx != nil { - dbtx.Rollback() + _ = dbtx.Rollback() } }() diff --git a/wallet/notifications.go b/wallet/notifications.go index fd943fa..af8e2b2 100644 --- a/wallet/notifications.go +++ b/wallet/notifications.go @@ -10,7 +10,6 @@ import ( "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/txscript" - "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcutil" "github.com/btcsuite/btcwallet/waddrmgr" "github.com/btcsuite/btcwallet/walletdb" @@ -184,7 +183,7 @@ func flattenBalanceMap(m map[uint32]btcutil.Amount) []AccountBalance { return s } -func relevantAccounts(w *Wallet, m map[uint32]btcutil.Amount, txs []TransactionSummary) { +func relevantAccounts(_ *Wallet, m map[uint32]btcutil.Amount, txs []TransactionSummary) { for _, tx := range txs { for _, d := range tx.MyInputs { m[d.PreviousAccount] = 0 @@ -255,7 +254,7 @@ func (s *NotificationServer) notifyMinedTransaction(dbtx walletdb.ReadTx, detail } txs := s.currentTxNtfn.AttachedBlocks[n-1].Transactions s.currentTxNtfn.AttachedBlocks[n-1].Transactions = - append(txs, makeTxSummary(dbtx, s.wallet, details)) + append(txs, makeTxSummary(dbtx, s.wallet, details)) // nolint:gocritic } func (s *NotificationServer) notifyAttachedBlock(dbtx walletdb.ReadTx, block *wtxmgr.BlockMeta) { @@ -493,27 +492,6 @@ func (s *NotificationServer) notifyUnspentOutput(account uint32, hash *chainhash } } -// notifySpentOutput notifies registered clients that a previously-unspent -// output is now spent, and includes the spender hash and input index in the -// notification. -func (s *NotificationServer) notifySpentOutput(account uint32, op *wire.OutPoint, spenderHash *chainhash.Hash, spenderIndex uint32) { - defer s.mu.Unlock() - s.mu.Lock() - clients := s.spentness[account] - if len(clients) == 0 { - return - } - n := &SpentnessNotifications{ - hash: &op.Hash, - index: op.Index, - spenderHash: spenderHash, - spenderIndex: spenderIndex, - } - for _, c := range clients { - c <- n - } -} - // SpentnessNotificationsClient receives SpentnessNotifications from the // NotificationServer over the channel C. type SpentnessNotificationsClient struct { diff --git a/wallet/recovery_test.go b/wallet/recovery_test.go index d5fc60d..efe31ff 100644 --- a/wallet/recovery_test.go +++ b/wallet/recovery_test.go @@ -65,7 +65,7 @@ type ( // delta. // // NOTE: This should be used before applying any CheckDelta steps. -func (_ InitialDelta) Apply(i int, h *Harness) { +func (InitialDelta) Apply(i int, h *Harness) { curHorizon, delta := h.brs.ExtendHorizon() assertHorizon(h.t, i, curHorizon, h.expHorizon) assertDelta(h.t, i, delta, h.recoveryWindow) diff --git a/wallet/signer_test.go b/wallet/signer_test.go index 0c0cd00..7ee2e48 100644 --- a/wallet/signer_test.go +++ b/wallet/signer_test.go @@ -34,6 +34,7 @@ func TestComputeInputScript(t *testing.T) { defer cleanup() for _, tc := range testCases { + tc := tc t.Run(tc.name, func(t *testing.T) { runTestCase(t, w, tc.scope, tc.expectedScriptLen) }) diff --git a/wallet/unstable.go b/wallet/unstable.go index c34b6d1..31f6889 100644 --- a/wallet/unstable.go +++ b/wallet/unstable.go @@ -17,10 +17,10 @@ type unstableAPI struct { // UnstableAPI exposes additional unstable public APIs for a Wallet. These APIs // may be changed or removed at any time. Currently this type exists to ease -// the transation (particularly for the legacy JSON-RPC server) from using +// the transition (particularly for the legacy JSON-RPC server) from using // exported manager packages to a unified wallet package that exposes all // functionality by itself. New code should not be written using this API. -func UnstableAPI(w *Wallet) unstableAPI { return unstableAPI{w} } +func UnstableAPI(w *Wallet) unstableAPI { return unstableAPI{w} } // nolint:golint // TxDetails calls wtxmgr.Store.TxDetails under a single database view transaction. func (u unstableAPI) TxDetails(txHash *chainhash.Hash) (*wtxmgr.TxDetails, error) { diff --git a/wallet/wallet.go b/wallet/wallet.go index 8c932da..c571ce6 100644 --- a/wallet/wallet.go +++ b/wallet/wallet.go @@ -45,8 +45,6 @@ const ( // data in the waddrmgr namespace. Transactions are not yet encrypted. InsecurePubPassphrase = "public" - walletDbWatchingOnlyName = "wowallet.db" - // recoveryBatchSize is the default number of blocks that will be // scanned successively by the recovery manager, in the event that the // wallet is started in recovery mode. @@ -120,11 +118,6 @@ type Wallet struct { changePassphrase chan changePassphraseRequest changePassphrases chan changePassphrasesRequest - // Information for reorganization handling. - reorganizingLock sync.Mutex - reorganizeToHash chainhash.Hash - reorganizing bool - NtfnServer *NotificationServer chainParams *chaincfg.Params @@ -606,7 +599,7 @@ func locateBirthdayBlock(chainClient chainConn, if mid == startHeight || mid == bestHeight || mid == left { birthdayBlock = &waddrmgr.BlockStamp{ Hash: *hash, - Height: int32(mid), + Height: mid, Timestamp: header.Timestamp, } break @@ -628,7 +621,7 @@ func locateBirthdayBlock(chainClient chainConn, birthdayBlock = &waddrmgr.BlockStamp{ Hash: *hash, - Height: int32(mid), + Height: mid, Timestamp: header.Timestamp, } break @@ -837,6 +830,7 @@ expandHorizons: // Update the global set of watched outpoints with any that were found // in the block. for outPoint, addr := range filterResp.FoundOutPoints { + outPoint := outPoint recoveryState.AddWatchedOutPoint(&outPoint, addr) } @@ -1223,7 +1217,7 @@ type ( // heldUnlock is a tool to prevent the wallet from automatically // locking after some timeout before an operation which needed - // the unlocked wallet has finished. Any aquired heldUnlock + // the unlocked wallet has finished. Any acquired heldUnlock // *must* be released (preferably with a defer) or the wallet // will forever remain unlocked. heldUnlock chan struct{} @@ -1425,25 +1419,6 @@ func (w *Wallet) ChangePassphrases(publicOld, publicNew, privateOld, return <-err } -// accountUsed returns whether there are any recorded transactions spending to -// a given account. It returns true if atleast one address in the account was -// used and false if no address in the account was used. -func (w *Wallet) accountUsed(addrmgrNs walletdb.ReadWriteBucket, account uint32) (bool, error) { - var used bool - err := w.Manager.ForEachAccountAddress(addrmgrNs, account, - func(maddr waddrmgr.ManagedAddress) error { - used = maddr.Used(addrmgrNs) - if used { - return waddrmgr.Break - } - return nil - }) - if err == waddrmgr.Break { - err = nil - } - return used, err -} - // AccountAddresses returns the addresses for every created address for an // account. func (w *Wallet) AccountAddresses(account uint32) (addrs []btcutil.Address, err error) { @@ -1802,8 +1777,6 @@ func (w *Wallet) RenameAccount(scope waddrmgr.KeyScope, account uint32, newName return err } -const maxEmptyAccounts = 100 - // NextAccount creates the next account and returns its account number. The // name must be unique to the account. In order to support automatic seed // restoring, new accounts may not be created when all of the previous 100 @@ -2024,8 +1997,12 @@ func (w *Wallet) ListSinceBlock(start, end, syncHeight int32) ([]btcjson.ListTra rangeFn := func(details []wtxmgr.TxDetails) (bool, error) { for _, detail := range details { - jsonResults := listTransactions(tx, &detail, - w.Manager, syncHeight, w.chainParams) + detail := detail + + jsonResults := listTransactions( + tx, &detail, w.Manager, syncHeight, + w.chainParams, + ) txList = append(txList, jsonResults...) } return false, nil @@ -2123,9 +2100,6 @@ func (w *Wallet) ListAddressTransactions(pkHashes map[string]struct{}) ([]btcjso jsonResults := listTransactions(tx, detail, w.Manager, syncBlock.Height, w.chainParams) - if err != nil { - return false, err - } txList = append(txList, jsonResults...) continue loopDetails } @@ -2874,7 +2848,7 @@ func (w *Wallet) SortedActivePaymentAddresses() ([]string, error) { return nil, err } - sort.Sort(sort.StringSlice(addrStrs)) + sort.Strings(addrStrs) return addrStrs, nil } diff --git a/wallet/wallet_test.go b/wallet/wallet_test.go index 5be8d74..8490699 100644 --- a/wallet/wallet_test.go +++ b/wallet/wallet_test.go @@ -73,6 +73,7 @@ func TestLocateBirthdayBlock(t *testing.T) { } for _, testCase := range testCases { + testCase := testCase success := t.Run(testCase.name, func(t *testing.T) { chainConn := createMockChainConn( chainParams.GenesisBlock, numBlocks, blockInterval, diff --git a/walletsetup.go b/walletsetup.go index c70425e..e317741 100644 --- a/walletsetup.go +++ b/walletsetup.go @@ -11,7 +11,6 @@ import ( "path/filepath" "time" - "github.com/btcsuite/btcd/btcec" "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcutil" @@ -30,7 +29,7 @@ func networkDir(dataDir string, chainParams *chaincfg.Params) string { // For now, we must always name the testnet data directory as "testnet" // and not "testnet3" or any other version, as the chaincfg testnet3 - // paramaters will likely be switched to being named "testnet3" in the + // parameters will likely be switched to being named "testnet3" in the // future. This is done to future proof that change, and an upgrade // plan to move the testnet3 data directory can be worked out later. if chainParams.Net == wire.TestNet3 { @@ -43,7 +42,7 @@ func networkDir(dataDir string, chainParams *chaincfg.Params) string { // convertLegacyKeystore converts all of the addresses in the passed legacy // key store to the new waddrmgr.Manager format. Both the legacy keystore and // the new manager must be unlocked. -func convertLegacyKeystore(legacyKeyStore *keystore.Store, w *wallet.Wallet) error { +func convertLegacyKeystore(legacyKeyStore *keystore.Store, w *wallet.Wallet) { netParams := legacyKeyStore.Net() blockStamp := waddrmgr.BlockStamp{ Height: 0, @@ -60,8 +59,9 @@ func convertLegacyKeystore(legacyKeyStore *keystore.Store, w *wallet.Wallet) err continue } - wif, err := btcutil.NewWIF((*btcec.PrivateKey)(privKey), - netParams, addr.Compressed()) + wif, err := btcutil.NewWIF( + privKey, netParams, addr.Compressed(), + ) if err != nil { fmt.Printf("WARN: Failed to create wallet "+ "import format for address %v: %v\n", @@ -93,8 +93,6 @@ func convertLegacyKeystore(legacyKeyStore *keystore.Store, w *wallet.Wallet) err continue } } - - return nil } // createWallet prompts the user for information needed to generate a new wallet @@ -114,7 +112,7 @@ func createWallet(cfg *config) error { var legacyKeyStore *keystore.Store _, err := os.Stat(keystorePath) if err != nil && !os.IsNotExist(err) { - // A stat error not due to a non-existant file should be + // A stat error not due to a non-existent file should be // returned to the caller. return err } else if err == nil { @@ -146,7 +144,7 @@ func createWallet(cfg *config) error { // Import the addresses in the legacy keystore to the new wallet if // any exist, locking each wallet again when finished. loader.RunAfterLoad(func(w *wallet.Wallet) { - defer legacyKeyStore.Lock() + defer func() { _ = legacyKeyStore.Lock() }() fmt.Println("Importing addresses from existing wallet...") @@ -161,12 +159,7 @@ func createWallet(cfg *config) error { return } - err = convertLegacyKeystore(legacyKeyStore, w) - if err != nil { - fmt.Printf("ERR: Failed to import keys from old "+ - "wallet format: %v", err) - return - } + convertLegacyKeystore(legacyKeyStore, w) // Remove the legacy key store. err = os.Remove(keystorePath) From 8d9d0b900174909d0cb34b5c1bd1fe24c40cafeb Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Wed, 24 Mar 2021 13:47:22 +0100 Subject: [PATCH 5/6] Travis: use make commands instead of goclean.sh Now that we have a Makefile with all tasks previously executed in goclean.sh, we no longer need to use that file. --- .gitignore | 1 + .travis.yml | 9 +++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index a5c7e6b..e383d01 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ btcwallet vendor .idea +coverage.txt diff --git a/.travis.yml b/.travis.yml index 3c204e3..ff1946b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,10 +4,11 @@ go: sudo: false install: - GO111MODULE=on go install -v ./... - - GO111MODULE=off go get -v golang.org/x/tools/cmd/cover - - GO111MODULE=off go get -v golang.org/x/lint/golint - - GO111MODULE=off go get -v github.com/davecgh/go-spew/spew script: - export PATH=$PATH:$HOME/gopath/bin - export GO111MODULE=on - - ./goclean.sh + - make fmt + - make lint + - make unit-race + - make unit-cover + - make goveralls From 371f7a583d9b6b704f19bcc3e50002d270b07094 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Wed, 24 Mar 2021 13:48:17 +0100 Subject: [PATCH 6/6] appveyor: remove appveyor configuration This configuration to run the checks on appveyor looks very old and unused since it still refers to glide for dependency management. We remove the file as it no longer serves a purpose. --- appveyor.yml | 22 ---------------------- 1 file changed, 22 deletions(-) delete mode 100644 appveyor.yml diff --git a/appveyor.yml b/appveyor.yml deleted file mode 100644 index 677a7cc..0000000 --- a/appveyor.yml +++ /dev/null @@ -1,22 +0,0 @@ -version: "{build}" - -clone_folder: c:\projects\src\github.com\btcsuite\btcwallet - -environment: - PATH: c:\projects\bin;%PATH% - GOPATH: c:\projects - GORACE: halt_on_error=1 - -init: - - go version - - go get -u github.com/Masterminds/glide - - go get -u github.com/davecgh/go-spew/spew - -install: - - glide install - -build_script: - - ps: go get $(glide novendor) - -test_script: - - ps: go test -race $(glide novendor)