Check every error.

This change is the result of using the errcheck tool
(https://github.com/kisielk/errcheck) to find all unchecked errors,
both unassigned and those assigned to the blank identifier.

Every returned error is now handled in some manner.  These include:

  - Logging errors that would otherwise be missed
  - Returning errors to the caller for further processing
  - Checking error values to determine what to do next
  - Panicking for truely exceptional "impossible" errors

On the subject of panics, they are a sharp tool and should be used
sparingly.  That being said, I have added them to check errors that
were previously explicitly ignored, because they were expected to
always return without failure.  This could be due to fake error paths
(i.e. writing to a bytes.Buffer panics for OOM and should never return
an error) or previous logic asserts that an error case is impossible.
Rather than leaving these unhandled and letting code fail later,
either with incorrect results or a nil pointer dereference, it now
produces a stack trace at the error emit site, which I find far more
useful when debugging.

While here, a bunch of dead code was removed, including code to move
pre-0.1.1 uxto and transaction history account files to the new
directory (as they would be unreadable anyways) and a big chunk of
commented out rpcclient code.

account.go

@@ -82,12 +82,9 @@ func (a *Account) AddressUsed(addr btcutil.Address) bool {
 	for _, r := range a.TxStore.Records() {
 		credits := r.Credits()
 		for _, c := range credits {
-			// Extract addresses from this output's pkScript.
+			// Errors don't matter here.  If addrs is nil, the
-			_, addrs, _, err := c.Addresses(activeNet.Params)
+			// range below does nothing.
-			if err != nil {
+			_, addrs, _, _ := c.Addresses(activeNet.Params)
-				continue
-			}
-
 			for _, a := range addrs {
 				if bytes.Equal(a.ScriptAddress(), pkHash) {
 					return true
@@ -503,16 +500,25 @@ func (a *Account) RescanActiveJob() (*RescanJob, error) {
 	return job, nil
 }
 
+// ResendUnminedTxs iterates through all transactions that spend from wallet
+// credits that are not known to have been mined into a block, and attempts
+// to send each to the chain server for relay.
 func (a *Account) ResendUnminedTxs() {
 	txs := a.TxStore.UnminedDebitTxs()
 	txbuf := new(bytes.Buffer)
 	for _, tx := range txs {
-		tx.MsgTx().Serialize(txbuf)
+		if err := tx.MsgTx().Serialize(txbuf); err != nil {
+			// Writing to a bytes.Buffer panics for OOM, and should
+			// not return any other errors.
+			panic(err)
+		}
 		hextx := hex.EncodeToString(txbuf.Bytes())
 		txsha, err := SendRawTransaction(CurrentServerConn(), hextx)
 		if err != nil {
 			// TODO(jrick): Check error for if this tx is a double spend,
 			// remove it if so.
+			log.Warnf("Could not resend transaction %v: %v",
+				txsha, err)
 		} else {
 			log.Debugf("Resent unmined transaction %v", txsha)
 		}

acctmgr.go

@@ -187,7 +187,6 @@ func openSavedAccount(name string, cfg *config) (*Account, error) {
 
 	wfilepath := accountFilename("wallet.bin", name, netdir)
 	txfilepath := accountFilename("tx.bin", name, netdir)
-	var wfile, txfile *os.File
 
 	// Read wallet file.
 	wfile, err := os.Open(wfilepath)
@@ -199,7 +198,11 @@ func openSavedAccount(name string, cfg *config) (*Account, error) {
 		msg := fmt.Sprintf("cannot open wallet file: %s", err)
 		return nil, &walletOpenError{msg}
 	}
-	defer wfile.Close()
+	defer func() {
+		if err := wfile.Close(); err != nil {
+			log.Warnf("Cannot close wallet file: %v", err)
+		}
+	}()
 
 	if _, err = wlt.ReadFrom(wfile); err != nil {
 		msg := fmt.Sprintf("cannot read wallet: %s", err)
@@ -209,21 +212,22 @@ func openSavedAccount(name string, cfg *config) (*Account, error) {
 	// Read tx file.  If this fails, return a errNoTxs error and let
 	// the caller decide if a rescan is necessary.
 	var finalErr error
-	if txfile, err = os.Open(txfilepath); err != nil {
+	txfile, err := os.Open(txfilepath)
+	if err != nil {
 		log.Errorf("cannot open tx file: %s", err)
-		// This is not a error we should immediately return with,
-		// but other errors can be more important, so only return
-		// this if none of the others are hit.
-		finalErr = errNoTxs
 		a.fullRescan = true
-	} else {
+		return a, errNoTxs
-		defer txfile.Close()
+	}
+	defer func() {
+		if err := txfile.Close(); err != nil {
+			log.Warnf("Cannot close txstore file: %v", err)
+		}
+	}()
 	if _, err = txs.ReadFrom(txfile); err != nil {
 		log.Errorf("cannot read tx file: %s", err)
 		a.fullRescan = true
 		finalErr = errNoTxs
 	}
-	}
 
 	return a, finalErr
 }
@@ -272,7 +276,11 @@ func openAccounts() *accountData {
 		log.Errorf("Unable to open account directory: %v", err)
 		return ad
 	}
-	defer accountDir.Close()
+	defer func() {
+		if err := accountDir.Close(); err != nil {
+			log.Warnf("Cannot close account directory")
+		}
+	}()
 	fileNames, err := accountDir.Readdirnames(0)
 	if err != nil {
 		// fileNames might be partially set, so log an error and
@@ -322,7 +330,10 @@ func (am *AccountManager) accountHandler() {
 		case *openAccountsCmd:
 			// Write all old accounts before proceeding.
 			for _, a := range ad.nameToAccount {
-				am.ds.FlushAccount(a)
+				if err := am.ds.FlushAccount(a); err != nil {
+					log.Errorf("Cannot write previously "+
+						"scheduled account file: %v", err)
+				}
 			}
 
 			ad = openAccounts()
@@ -355,7 +366,6 @@ func (am *AccountManager) accountHandler() {
 		case *markAddressForAccountCmd:
 			// TODO(oga) make sure we own account
 			ad.addressToAccount[cmd.address] = cmd.account
-
 		}
 	}
 }
@@ -536,13 +546,16 @@ func (am *AccountManager) RegisterNewAccount(a *Account) error {
 
 // Rollback rolls back each managed Account to the state before the block
 // specified by height and hash was connected to the main chain.
-func (am *AccountManager) Rollback(height int32, hash *btcwire.ShaHash) {
+func (am *AccountManager) Rollback(height int32, hash *btcwire.ShaHash) error {
 	log.Infof("Rolling back tx history since block height %v", height)
 
 	for _, a := range am.AllAccounts() {
-		a.TxStore.Rollback(height)
+		if err := a.TxStore.Rollback(height); err != nil {
+			return err
+		}
 		am.ds.ScheduleTxStoreWrite(a)
 	}
+	return nil
 }
 
 // BlockNotify notifies all frontends of any changes from the new block,
@@ -645,7 +658,7 @@ func (am *AccountManager) ChangePassphrase(old, new []byte) error {
 	accts := am.AllAccounts()
 
 	for _, a := range accts {
-		if locked := a.Wallet.IsLocked(); !locked {
+		if !a.IsLocked() {
 			if err := a.Wallet.Lock(); err != nil {
 				return err
 			}
@@ -654,7 +667,11 @@ func (am *AccountManager) ChangePassphrase(old, new []byte) error {
 		if err := a.Wallet.Unlock(old); err != nil {
 			return err
 		}
-		defer a.Wallet.Lock()
+		defer func() {
+			if err := a.Lock(); err != nil {
+				log.Warnf("Cannot lock account: %v", err)
+			}
+		}()
 	}
 
 	// Change passphrase for each unlocked wallet.
@@ -681,22 +698,32 @@ func (am *AccountManager) LockWallets() error {
 
 // UnlockWallets unlocks all managed account's wallets.  If any wallet unlocks
 // fail, all successfully unlocked wallets are locked again.
-func (am *AccountManager) UnlockWallets(passphrase string) error {
+func (am *AccountManager) UnlockWallets(passphrase string) (err error) {
 	accts := am.AllAccounts()
+
 	unlockedAccts := make([]*Account, 0, len(accts))
+	defer func() {
+		// Lock all account wallets unlocked during this call
+		// if any of the unlocks failed.
+		if err != nil {
+			for _, ua := range unlockedAccts {
+				if err := ua.Lock(); err != nil {
+					log.Warnf("Cannot lock account '%s': %v",
+						ua.name, err)
+				}
+			}
+		}
+	}()
 
 	for _, a := range accts {
-		if err := a.Unlock([]byte(passphrase)); err != nil {
+		if uErr := a.Unlock([]byte(passphrase)); uErr != nil {
-			for _, ua := range unlockedAccts {
+			err = fmt.Errorf("cannot unlock account %v: %v",
-				ua.Lock()
+				a.name, uErr)
-			}
+			return
-			return fmt.Errorf("cannot unlock account %v: %v",
-				a.name, err)
 		}
 		unlockedAccts = append(unlockedAccts, a)
 	}
-
+	return
-	return nil
 }
 
 // DumpKeys returns all WIF-encoded private keys associated with all

cmd.go

@@ -17,7 +17,6 @@
 package main
 
 import (
-	"errors"
 	"github.com/conformal/btcjson"
 	"github.com/conformal/btcutil"
 	"github.com/conformal/btcwallet/wallet"
@@ -47,19 +46,19 @@ var (
 // GetCurBlock returns the blockchain height and SHA hash of the most
 // recently seen block.  If no blocks have been seen since btcd has
 // connected, btcd is queried for the current block height and hash.
-func GetCurBlock() (bs wallet.BlockStamp, err error) {
+func GetCurBlock() (wallet.BlockStamp, error) {
 	curBlock.RLock()
-	bs = curBlock.BlockStamp
+	bs := curBlock.BlockStamp
 	curBlock.RUnlock()
 	if bs.Height != int32(btcutil.BlockHeightUnknown) {
 		return bs, nil
 	}
 
-	bb, _ := GetBestBlock(CurrentServerConn())
+	bb, jsonErr := GetBestBlock(CurrentServerConn())
-	if bb == nil {
+	if jsonErr != nil {
 		return wallet.BlockStamp{
 			Height: int32(btcutil.BlockHeightUnknown),
-		}, errors.New("current block unavailable")
+		}, jsonErr
 	}
 
 	hash, err := btcwire.NewShaHashFromStr(bb.Hash)

createtx.go

@@ -233,6 +233,8 @@ func (a *Account) txToPairs(pairs map[string]btcutil.Amount,
 			msgtx.AddTxIn(btcwire.NewTxIn(ip.OutPoint(), nil))
 		}
 		for i, input := range inputs {
+			// Errors don't matter here, as we only consider the
+			// case where len(addrs) == 1.
 			_, addrs, _, _ := input.Addresses(activeNet.Params)
 			if len(addrs) != 1 {
 				continue
@@ -296,7 +298,11 @@ func (a *Account) txToPairs(pairs map[string]btcutil.Amount,
 
 	buf := bytes.NewBuffer(nil)
 	buf.Grow(msgtx.SerializeSize())
-	msgtx.BtcEncode(buf, btcwire.ProtocolVersion)
+	if err := msgtx.BtcEncode(buf, btcwire.ProtocolVersion); err != nil {
+		// Hitting OOM by growing or writing to a bytes.Buffer already
+		// panics, and all returned errors are unexpected.
+		panic(err)
+	}
 	info := &CreatedTx{
 		tx:         btcutil.NewTx(msgtx),
 		inputs:     selectedInputs,

disksync.go

@@ -62,7 +62,11 @@ func freshDir(path string) error {
 	if err != nil {
 		return err
 	}
-	defer fd.Close()
+	defer func() {
+		if err := fd.Close(); err != nil {
+			log.Warnf("Cannot close directory: %v", err)
+		}
+	}()
 	names, err := fd.Readdirnames(0)
 	if err != nil {
 		return err
@@ -374,13 +378,11 @@ func (a *Account) writeWallet(dir string) error {
 	}
 
 	tmppath := tmpfile.Name()
-	tmpfile.Close()
+	if err := tmpfile.Close(); err != nil {
-
+		log.Warnf("Cannot close temporary wallet file: %v", err)
-	if err = Rename(tmppath, wfilepath); err != nil {
-		return err
 	}
 
-	return nil
+	return Rename(tmppath, wfilepath)
 }
 
 func (a *Account) writeTxStore(dir string) error {
@@ -396,11 +398,9 @@ func (a *Account) writeTxStore(dir string) error {
 	}
 
 	tmppath := tmpfile.Name()
-	tmpfile.Close()
+	if err := tmpfile.Close(); err != nil {
-
+		log.Warnf("Cannot close temporary txstore file: %v", err)
-	if err = Rename(tmppath, txfilepath); err != nil {
-		return err
 	}
 
-	return nil
+	return Rename(tmppath, txfilepath)
 }

ntfns.go

@@ -108,6 +108,8 @@ func NtfnRecvTx(n btcjson.Cmd) error {
 	// and record the received txout.
 	for outIdx, txout := range tx.MsgTx().TxOut {
 		var accounts []*Account
+		// Errors don't matter here.  If addrs is nil, the range below
+		// does nothing.
 		_, addrs, _, _ := btcscript.ExtractPkScriptAddrs(txout.PkScript,
 			activeNet.Params)
 		for _, addr := range addrs {
@@ -206,7 +208,11 @@ func NtfnBlockConnected(n btcjson.Cmd) error {
 	AcctMgr.BlockNotify(bs)
 
 	// Pass notification to frontends too.
-	marshaled, _ := n.MarshalJSON()
+	marshaled, err := n.MarshalJSON()
+	// The parsed notification is expected to be marshalable.
+	if err != nil {
+		panic(err)
+	}
 	allClients <- marshaled
 
 	return nil
@@ -226,10 +232,18 @@ func NtfnBlockDisconnected(n btcjson.Cmd) error {
 	}
 
 	// Rollback Utxo and Tx data stores.
-	AcctMgr.Rollback(bdn.Height, hash)
+	if err = AcctMgr.Rollback(bdn.Height, hash); err != nil {
+		return err
+	}
 
 	// Pass notification to frontends too.
-	marshaled, _ := n.MarshalJSON()
+	marshaled, err := n.MarshalJSON()
+	// A btcws.BlockDisconnectedNtfn is expected to marshal without error.
+	// If it does, it indicates that one of its struct fields is of a
+	// non-marshalable type.
+	if err != nil {
+		panic(err)
+	}
 	allClients <- marshaled
 
 	return nil

rpcclient.go

@@ -131,8 +131,14 @@ type AddRPCRequest struct {
 // websocket connection.
 func (btcd *BtcdRPCConn) send(rpcrequest *ServerRequest) error {
 	// btcjson.Cmds define their own MarshalJSON which returns an error
-	// to satisify the json.Marshaler interface, but will never error.
+	// to satisify the json.Marshaler interface, but should never error.
-	mrequest, _ := rpcrequest.request.MarshalJSON()
+	// If an error does occur, it is due to a struct containing a type
+	// that is not marshalable, so panic here rather than silently
+	// ignoring it.
+	mrequest, err := rpcrequest.request.MarshalJSON()
+	if err != nil {
+		panic(err)
+	}
 	return websocket.Message.Send(btcd.ws, mrequest)
 }
 
@@ -155,7 +161,10 @@ func (btcd *BtcdRPCConn) Start() {
 					// Connection lost.
 					log.Infof("Cannot complete btcd websocket send: %v",
 						err)
-					btcd.ws.Close()
+					if err := btcd.ws.Close(); err != nil {
+						log.Warnf("Cannot close btcd "+
+							"websocket connection: %v", err)
+					}
 					close(done)
 				}
 
@@ -176,42 +185,6 @@ func (btcd *BtcdRPCConn) Start() {
 
 				rpcrequest.response <- rawResponse
 
-				/*
-					//rpcrequest.result
-					var jsonErr *btcjson.Error
-					if rawResponse.result != nil {
-					}
-					err := json.Unmarshal([]byte(*rawResponse.result), &result)
-					err := json.Unmarshal([]byte(*rawResponse.error), &error)
-
-					rawResult := recvResponse
-					rawError := recvResponse
-					response := &ServerResponse{
-						result: r.Result,
-						err:    jsonErr
-					}
-					rpcrequest.response <- response
-				*/
-
-				/*
-					// If no result var was set, create and send
-					// send the response unmarshaled by the json
-					// package.
-
-
-					if rpcrequest.result == nil {
-						response := &ServerResponse{
-							result: recvResponse.reply.Result,
-							err:    recvResponse.reply.Error,
-						}
-						rpcrequest.response <- response
-						continue
-					}
-
-					// A return var was set, so unmarshal again
-					// into the var before sending the response.
-				*/
-
 			case <-done:
 				resp := RawRPCResponse{Error: &ErrBtcdDisconnectedRaw}
 				for _, request := range m {
@@ -237,7 +210,10 @@ func (btcd *BtcdRPCConn) Start() {
 					log.Infof("Cannot receive btcd websocket message: %v",
 						err)
 				}
-				btcd.ws.Close()
+				if err := btcd.ws.Close(); err != nil {
+					log.Warnf("Cannot close btcd "+
+						"websocket connection: %v", err)
+				}
 				close(responses)
 				return
 			}
@@ -307,8 +283,12 @@ func GetBestBlock(rpc ServerConn) (*btcws.GetBestBlockResult, *btcjson.Error) {
 
 // GetBlock requests details about a block with the given hash.
 func GetBlock(rpc ServerConn, blockHash string) (*btcjson.BlockResult, *btcjson.Error) {
-	// NewGetBlockCmd cannot fail with no optargs, so omit the check.
+	// NewGetBlockCmd should never fail with no optargs.  If this does fail,
-	cmd, _ := btcjson.NewGetBlockCmd(<-NewJSONID, blockHash)
+	// panic now rather than later.
+	cmd, err := btcjson.NewGetBlockCmd(<-NewJSONID, blockHash)
+	if err != nil {
+		panic(err)
+	}
 	response := <-rpc.SendRequest(NewServerRequest(cmd))
 
 	var resultData btcjson.BlockResult
@@ -367,12 +347,16 @@ func NotifySpent(rpc ServerConn, outpoints []*btcwire.OutPoint) *btcjson.Error {
 func Rescan(rpc ServerConn, beginBlock int32, addrs []string,
 	outpoints []*btcwire.OutPoint) *btcjson.Error {
 
-	// NewRescanCmd cannot fail with no optargs, so omit the check.
 	ops := make([]btcws.OutPoint, len(outpoints))
 	for i := range outpoints {
 		ops[i] = *btcws.NewOutPointFromWire(outpoints[i])
 	}
-	cmd, _ := btcws.NewRescanCmd(<-NewJSONID, beginBlock, addrs, ops)
+	// NewRescanCmd should never fail with no optargs.  If this does fail,
+	// panic now rather than later.
+	cmd, err := btcws.NewRescanCmd(<-NewJSONID, beginBlock, addrs, ops)
+	if err != nil {
+		panic(err)
+	}
 	response := <-rpc.SendRequest(NewServerRequest(cmd))
 	_, jsonErr := response.FinishUnmarshal(nil)
 	return jsonErr
@@ -380,8 +364,12 @@ func Rescan(rpc ServerConn, beginBlock int32, addrs []string,
 
 // SendRawTransaction sends a hex-encoded transaction for relay.
 func SendRawTransaction(rpc ServerConn, hextx string) (txid string, error *btcjson.Error) {
-	// NewSendRawTransactionCmd cannot fail, so omit the check.
+	// NewSendRawTransactionCmd should never fail.  In the exceptional case
-	cmd, _ := btcjson.NewSendRawTransactionCmd(<-NewJSONID, hextx)
+	// where it does, panic here rather than later.
+	cmd, err := btcjson.NewSendRawTransactionCmd(<-NewJSONID, hextx)
+	if err != nil {
+		panic(err)
+	}
 	response := <-rpc.SendRequest(NewServerRequest(cmd))
 
 	var resultData string
@@ -396,9 +384,12 @@ func SendRawTransaction(rpc ServerConn, hextx string) (txid string, error *btcjs
 // command for txsha.. When the result of the request is required it may be
 // collected with GetRawTRansactionAsyncResult.
 func GetRawTransactionAsync(rpc ServerConn, txsha *btcwire.ShaHash) chan RawRPCResponse {
-	// NewGetRawTransactionCmd cannot fail with no optargs.
+	// NewGetRawTransactionCmd should never fail with no optargs.  If this
-	cmd, _ := btcjson.NewGetRawTransactionCmd(<-NewJSONID, txsha.String())
+	// does fail, panic now rather than later.
-
+	cmd, err := btcjson.NewGetRawTransactionCmd(<-NewJSONID, txsha.String())
+	if err != nil {
+		panic(err)
+	}
 	return rpc.SendRequest(NewServerRequest(cmd))
 }
 
@@ -436,8 +427,12 @@ func GetRawTransaction(rpc ServerConn, txsha *btcwire.ShaHash) (*btcutil.Tx, *bt
 // VerboseGetRawTransaction sends the verbose version of a getrawtransaction
 // request to receive details about a transaction.
 func VerboseGetRawTransaction(rpc ServerConn, txsha *btcwire.ShaHash) (*btcjson.TxRawResult, *btcjson.Error) {
-	// NewGetRawTransactionCmd cannot fail with a single optarg.
+	// NewGetRawTransactionCmd should never fail with a single optarg.  If
-	cmd, _ := btcjson.NewGetRawTransactionCmd(<-NewJSONID, txsha.String(), 1)
+	// it does, panic now rather than later.
+	cmd, err := btcjson.NewGetRawTransactionCmd(<-NewJSONID, txsha.String(), 1)
+	if err != nil {
+		panic(err)
+	}
 	response := <-rpc.SendRequest(NewServerRequest(cmd))
 
 	var resultData btcjson.TxRawResult

rpcserver.go

@@ -204,12 +204,25 @@ func WalletRequestProcessor() {
 			AcctMgr.Release()
 
 			if jsonErr != nil {
-				b, _ := json.Marshal(jsonErr)
+				b, err := json.Marshal(jsonErr)
+				// Marshal should only fail if jsonErr contains
+				// vars of an non-mashalable type, which would
+				// indicate a source code issue with btcjson.
+				if err != nil {
+					panic(err)
+				}
 				r.response <- RawRPCResponse{
 					Error: (*json.RawMessage)(&b),
 				}
 			} else {
-				b, _ := json.Marshal(result)
+				b, err := json.Marshal(result)
+				// Marshal should only fail if result contains
+				// vars of an unmashalable type.  This may
+				// indicate an bug with the calle RPC handler,
+				// and should be logged.
+				if err != nil {
+					log.Errorf("Cannot marshal result: %v", err)
+				}
 				r.response <- RawRPCResponse{
 					Result: (*json.RawMessage)(&b),
 				}
@@ -573,8 +586,11 @@ func GetBalance(icmd btcjson.Cmd) (interface{}, *btcjson.Error) {
 // exist.
 func GetInfo(icmd btcjson.Cmd) (interface{}, *btcjson.Error) {
 	// Call down to btcd for all of the information in this command known
-	// by them.  This call can not realistically ever fail.
+	// by them.  This call is expected to always succeed.
-	gicmd, _ := btcjson.NewGetInfoCmd(<-NewJSONID)
+	gicmd, err := btcjson.NewGetInfoCmd(<-NewJSONID)
+	if err != nil {
+		panic(err)
+	}
 	response := <-CurrentServerConn().SendRequest(NewServerRequest(gicmd))
 
 	var info btcjson.InfoResult
@@ -970,6 +986,8 @@ func GetTransaction(icmd btcjson.Cmd) (interface{}, *btcjson.Error) {
 			received += cred.Amount()
 
 			var addr string
+			// Errors don't matter here, as we only consider the
+			// case where len(addrs) == 1.
 			_, addrs, _, _ := cred.Addresses(activeNet.Params)
 			if len(addrs) == 1 {
 				addr = addrs[0].EncodeAddress()
@@ -1007,6 +1025,8 @@ func GetTransaction(icmd btcjson.Cmd) (interface{}, *btcjson.Error) {
 			Amount: (-debits.OutputAmount(true)).ToUnit(btcutil.AmountBTC),
 			Fee:    debits.Fee().ToUnit(btcutil.AmountBTC),
 		}
+		// Errors don't matter here, as we only consider the
+		// case where len(addrs) == 1.
 		_, addrs, _, _ := debitTx.Credits()[0].Addresses(activeNet.Params)
 		if len(addrs) == 1 {
 			info.Address = addrs[0].EncodeAddress()
@@ -1376,7 +1396,11 @@ func sendPairs(icmd btcjson.Cmd, account string, amounts map[string]btcutil.Amou
 
 	serializedTx := bytes.NewBuffer(nil)
 	serializedTx.Grow(createdTx.tx.MsgTx().SerializeSize())
-	createdTx.tx.MsgTx().Serialize(serializedTx)
+	if err := createdTx.tx.MsgTx().Serialize(serializedTx); err != nil {
+		// Hitting OOM writing to a bytes.Buffer already panics, and
+		// all other errors are unexpected.
+		panic(err)
+	}
 	hextx := hex.EncodeToString(serializedTx.Bytes())
 	txSha, jsonErr := SendRawTransaction(CurrentServerConn(), hextx)
 	if jsonErr != nil {
@@ -2042,9 +2066,11 @@ func SignRawTransaction(icmd btcjson.Cmd) (interface{}, *btcjson.Error) {
 	buf := bytes.NewBuffer(nil)
 	buf.Grow(msgTx.SerializeSize())
 
-	// Buffer is the right size, this should never fail so no need to
+	// All returned errors (not OOM, which panics) encounted during
-	// come up with some synthetic error code for it.
+	// bytes.Buffer writes are unexpected.
-	_ = msgTx.Serialize(buf)
+	if err = msgTx.Serialize(buf); err != nil {
+		panic(err)
+	}
 
 	return btcjson.SignRawTransactionResult{
 		Hex:      hex.EncodeToString(buf.Bytes()),
@@ -2075,9 +2101,12 @@ func ValidateAddress(icmd btcjson.Cmd) (interface{}, *btcjson.Error) {
 	// We can't use AcctMgr.Address() here since we also need the account
 	// name.
 	if account, err := AcctMgr.AccountByAddress(addr); err == nil {
-		// we ignore these errors because if this call passes this can't
+		// The address must be handled by this account, so we expect
-		// realistically fail.
+		// this call to succeed without error.
-		ainfo, _ := account.Address(addr)
+		ainfo, err := account.Address(addr)
+		if err != nil {
+			panic(err)
+		}
 
 		result.IsMine = true
 		result.Account = account.name
@@ -2231,8 +2260,11 @@ func WalletPassphrase(icmd btcjson.Cmd) (interface{}, *btcjson.Error) {
 	go func(timeout int64) {
 		time.Sleep(time.Second * time.Duration(timeout))
 		AcctMgr.Grab()
-		_ = AcctMgr.LockWallets()
+		defer AcctMgr.Release()
-		AcctMgr.Release()
+		err := AcctMgr.LockWallets()
+		if err != nil {
+			log.Warnf("Cannot lock account wallets: %v", err)
+		}
 	}(cmd.Timeout)
 
 	return nil, nil
@@ -2282,7 +2314,12 @@ type AccountNtfn struct {
 // that the wallet has just been locked or unlocked.
 func NotifyWalletLockStateChange(account string, locked bool) {
 	ntfn := btcws.NewWalletLockStateNtfn(account, locked)
-	mntfn, _ := ntfn.MarshalJSON()
+	mntfn, err := ntfn.MarshalJSON()
+	// If the marshal failed, it indicates that the btcws notification
+	// struct contains a field with a type that is not marshalable.
+	if err != nil {
+		panic(err)
+	}
 	allClients <- mntfn
 }
 
@@ -2290,7 +2327,12 @@ func NotifyWalletLockStateChange(account string, locked bool) {
 // to a frontend.
 func NotifyWalletBalance(frontend chan []byte, account string, balance float64) {
 	ntfn := btcws.NewAccountBalanceNtfn(account, balance, true)
-	mntfn, _ := ntfn.MarshalJSON()
+	mntfn, err := ntfn.MarshalJSON()
+	// If the marshal failed, it indicates that the btcws notification
+	// struct contains a field with a type that is not marshalable.
+	if err != nil {
+		panic(err)
+	}
 	frontend <- mntfn
 }
 
@@ -2298,7 +2340,12 @@ func NotifyWalletBalance(frontend chan []byte, account string, balance float64)
 // notification to a frontend.
 func NotifyWalletBalanceUnconfirmed(frontend chan []byte, account string, balance float64) {
 	ntfn := btcws.NewAccountBalanceNtfn(account, balance, false)
-	mntfn, _ := ntfn.MarshalJSON()
+	mntfn, err := ntfn.MarshalJSON()
+	// If the marshal failed, it indicates that the btcws notification
+	// struct contains a field with a type that is not marshalable.
+	if err != nil {
+		panic(err)
+	}
 	frontend <- mntfn
 }
 
@@ -2307,7 +2354,12 @@ func NotifyNewTxDetails(frontend chan []byte, account string,
 	details btcjson.ListTransactionsResult) {
 
 	ntfn := btcws.NewTxNtfn(account, &details)
-	mntfn, _ := ntfn.MarshalJSON()
+	mntfn, err := ntfn.MarshalJSON()
+	// If the marshal failed, it indicates that the btcws notification
+	// struct contains a field with a type that is not marshalable.
+	if err != nil {
+		panic(err)
+	}
 	frontend <- mntfn
 }
 

sockets.go

@@ -202,11 +202,13 @@ func genCertPair(certFile, keyFile string) error {
 		return err
 	}
 	if err = ioutil.WriteFile(keyFile, key, 0600); err != nil {
-		os.Remove(certFile)
+		if rmErr := os.Remove(certFile); rmErr != nil {
+			log.Warnf("Cannot remove written certificates: %v", rmErr)
+		}
 		return err
 	}
 
-	log.Infof("Done generating TLS certificates")
+	log.Info("Done generating TLS certificates")
 	return nil
 }
 
@@ -262,7 +264,14 @@ func (s *server) ReplyToFrontend(msg []byte, ws, authenticated bool) ([]byte, er
 			Id:    &id,
 			Error: jsonErr,
 		}
-		mresponse, _ := json.Marshal(response)
+		mresponse, err := json.Marshal(response)
+		// We expect the marshal to succeed.  If it doesn't, it
+		// indicates that either jsonErr (which is created by us) or
+		// the id itself (which was successfully unmashaled) are of
+		// some non-marshalable type.
+		if err != nil {
+			panic(err)
+		}
 		return mresponse, nil
 	}
 
@@ -282,12 +291,18 @@ func (s *server) ReplyToFrontend(msg []byte, ws, authenticated bool) ([]byte, er
 	}
 	mresponse, err := json.Marshal(response)
 	if err != nil {
-		log.Errorf("Cannot marhal response: %v", err)
+		log.Errorf("Cannot marshal response: %v", err)
 		response := btcjson.Reply{
 			Id:    &id,
 			Error: &btcjson.ErrInternal,
 		}
-		mresponse, _ = json.Marshal(&response)
+		mresponse, err = json.Marshal(&response)
+		// We expect this marshal to succeed.  If it doesn't, btcjson
+		// returned an id with an non-marshalable type or ErrInternal
+		// is just plain wrong.
+		if err != nil {
+			panic(err)
+		}
 	}
 
 	return mresponse, nil
@@ -337,10 +352,13 @@ func clientResponseDuplicator() {
 func NotifyBtcdConnection(reply chan []byte) {
 	if btcd, ok := CurrentServerConn().(*BtcdRPCConn); ok {
 		ntfn := btcws.NewBtcdConnectedNtfn(btcd.Connected())
-		mntfn, _ := ntfn.MarshalJSON()
+		mntfn, err := ntfn.MarshalJSON()
+		// btcws notifications must always marshal without error.
+		if err != nil {
+			panic(err)
+		}
 		reply <- mntfn
 	}
-
 }
 
 // stringQueue manages a queue of strings, reading from in and sending
@@ -409,7 +427,9 @@ out:
 func (s *server) WSSendRecv(ws *websocket.Conn, remoteAddr string, authenticated bool) {
 	// Clear the read deadline set before the websocket hijacked
 	// the connection.
-	ws.SetReadDeadline(time.Time{})
+	if err := ws.SetReadDeadline(time.Time{}); err != nil {
+		log.Warnf("Cannot remove read deadline: %v", err)
+	}
 
 	// Add client context so notifications duplicated to each
 	// client are received by this client.
@@ -535,7 +555,11 @@ out:
 // btcd, so this can probably be removed.
 func NotifyNewBlockChainHeight(reply chan []byte, bs wallet.BlockStamp) {
 	ntfn := btcws.NewBlockConnectedNtfn(bs.Hash.String(), bs.Height)
-	mntfn, _ := ntfn.MarshalJSON()
+	mntfn, err := ntfn.MarshalJSON()
+	// btcws notifications must always marshal without error.
+	if err != nil {
+		panic(err)
+	}
 	reply <- mntfn
 }
 
@@ -598,7 +622,10 @@ func (s *server) Start() {
 		s.wg.Add(1)
 		go func(listener net.Listener) {
 			log.Infof("RPCS: RPC server listening on %s", listener.Addr())
-			httpServer.Serve(listener)
+			if err := httpServer.Serve(listener); err != nil {
+				log.Errorf("Listener for %s exited with error: %v",
+					listener.Addr(), err)
+			}
 			log.Tracef("RPCS: RPC listener done for %s", listener.Addr())
 			s.wg.Done()
 		}(listener)
@@ -754,7 +781,10 @@ func Handshake(rpc ServerConn) error {
 		// try to write new tx and utxo files on each rollback.
 		if it.Next() {
 			bs := it.BlockStamp()
-			AcctMgr.Rollback(bs.Height, &bs.Hash)
+			err := AcctMgr.Rollback(bs.Height, &bs.Hash)
+			if err != nil {
+				return err
+			}
 		}
 
 		// Set default account to be marked in sync with the current
@@ -788,7 +818,9 @@ func Handshake(rpc ServerConn) error {
 	// about.
 	a.fullRescan = true
 	AcctMgr.Track()
-	AcctMgr.RescanActiveAddresses()
+	if err := AcctMgr.RescanActiveAddresses(); err != nil {
+		return err
+	}
 	// TODO: only begin tracking new unspent outputs as a result of the
 	// rescan.  This is also racy (see comment for second Track above).
 	AcctMgr.Track()

txstore/serialization.go

@@ -758,12 +758,14 @@ func (tx *msgTx) ReadFrom(r io.Reader) (int64, error) {
 }
 
 func (tx *msgTx) WriteTo(w io.Writer) (int64, error) {
-	// Write to a buffer and then copy to w so the total number
+	// Write to a buffer and then copy to w so the total number of bytes
-	// of bytes written can be returned to the caller.  Writing
+	// written can be returned to the caller.  Writing to a to a
-	// to a bytes.Buffer never fails except for OOM, so omit the
+	// bytes.Buffer never fails except for OOM panics, so check and panic 
-	// serialization error check.
+	// on any unexpected non-nil returned errors.
 	buf := new(bytes.Buffer)
-	_ = (*btcwire.MsgTx)(tx).Serialize(buf)
+	if err := (*btcwire.MsgTx)(tx).Serialize(buf); err != nil {
+		panic(err)
+	}
 	return io.Copy(w, buf)
 }
 

updates.go

@@ -61,12 +61,21 @@ func updateOldFileLocations() {
 	//
 	// Previous account files are placed in the testnet directory
 	// as 0.1.0 and earlier only ran on testnet.
+	//
+	// UTXOs and transaction history are intentionally not moved over, as
+	// the UTXO file is no longer used (it was combined with txstore), and
+	// the tx history is now written in an incompatible format and would
+	// be ignored on first read.
 
 	datafi, err := os.Open(cfg.DataDir)
 	if err != nil {
 		return
 	}
-	defer datafi.Close()
+	defer func() {
+		if err := datafi.Close(); err != nil {
+			log.Warnf("Cannot close data directory: %v", err)
+		}
+	}()
 
 	// Get info on all files in the data directory.
 	fi, err := datafi.Readdir(0)
@@ -131,30 +140,10 @@ func updateOldFileLocations() {
 			}
 		}
 
-		// Move old tx.bin, if any.
-		old = filepath.Join(cfg.DataDir, fi[i].Name(), "tx.bin")
-		if fileExists(old) {
-			new := accountFilename("tx.bin", account, netdir)
-			if err := Rename(old, new); err != nil {
-				log.Errorf("Cannot move old %v for account %v to new location: %v",
-					"tx.bin", account, err)
-				os.Exit(1)
-			}
-		}
-
-		// Move old utxo.bin, if any.
-		old = filepath.Join(cfg.DataDir, fi[i].Name(), "utxo.bin")
-		if fileExists(old) {
-			new := accountFilename("utxo.bin", account, netdir)
-			if err := Rename(old, new); err != nil {
-				log.Errorf("Cannot move old %v for account %v to new location: %v",
-					"utxo.bin", account, err)
-				os.Exit(1)
-			}
-		}
-
 		// Cleanup old account directory.
-		os.RemoveAll(filepath.Join(cfg.DataDir, fi[i].Name()))
+		if err := os.RemoveAll(filepath.Join(cfg.DataDir, fi[i].Name())); err != nil {
+			log.Warnf("Could not remove pre 0.1.1 account directory: %v", err)
+		}
 	}
 }
 

version.go

@@ -77,7 +77,12 @@ func normalizeVerString(str string) string {
 	var result bytes.Buffer
 	for _, r := range str {
 		if strings.ContainsRune(semanticAlphabet, r) {
-			result.WriteRune(r)
+			_, err := result.WriteRune(r)
+			// Writing to a bytes.Buffer panics on OOM, and all
+			// errors are unexpected.
+			if err != nil {
+				panic(err)
+			}
 		}
 	}
 	return result.String()