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/.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 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 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 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) 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/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" ) 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)