Implement BIP0061 reject handling (pver 70002).

This commit implements reject handling as defined by BIP0061 and bumps the
maximum supported protocol version to 70002 accordingly.

As a part of supporting this a new error type named RuleError has been
introduced which encapsulates and underlying error which could be one of
the existing TxRuleError or btcchain.RuleError types.

This allows a single high level type assertion to be used to determine if
the block or transaction was rejected due to a rule error or due to an
unexpected error.  Meanwhile, an appropriate reject error can be created
from the error by pulling the underlying error out and using it.

Also, a check for minimum protocol version of 209 has been added.

Closes #133.

blockmanager.go

@@ -499,12 +499,19 @@ func (b *blockManager) handleTxMsg(tmsg *txMsg) {
 		// simply rejected as opposed to something actually going wrong,
 		// so log it as such.  Otherwise, something really did go wrong,
 		// so log it as an actual error.
-		if _, ok := err.(TxRuleError); ok {
+		if _, ok := err.(RuleError); ok {
-			bmgrLog.Debugf("Rejected transaction %v from %s: %v", txHash,
+			bmgrLog.Debugf("Rejected transaction %v from %s: %v",
-				tmsg.peer, err)
+				txHash, tmsg.peer, err)
 		} else {
-			bmgrLog.Errorf("Failed to process transaction %v: %v", txHash, err)
+			bmgrLog.Errorf("Failed to process transaction %v: %v",
+				txHash, err)
 		}
+
+		// Convert the error into an appropriate reject message and
+		// send it.
+		code, reason := errToRejectErr(err)
+		tmsg.peer.PushRejectMsg(btcwire.CmdBlock, code, reason, txHash,
+			false)
 		return
 	}
 }
@@ -594,8 +601,15 @@ func (b *blockManager) handleBlockMsg(bmsg *blockMsg) {
 			bmgrLog.Infof("Rejected block %v from %s: %v", blockSha,
 				bmsg.peer, err)
 		} else {
-			bmgrLog.Errorf("Failed to process block %v: %v", blockSha, err)
+			bmgrLog.Errorf("Failed to process block %v: %v",
+				blockSha, err)
 		}
+
+		// Convert the error into an appropriate reject message and
+		// send it.
+		code, reason := errToRejectErr(err)
+		bmsg.peer.PushRejectMsg(btcwire.CmdBlock, code, reason,
+			blockSha, false)
 		return
 	}
 

log.go

@@ -7,6 +7,7 @@ package main
 import (
 	"fmt"
 	"os"
+	"strings"
 	"time"
 
 	"github.com/conformal/btcd/addrmgr"
@@ -26,6 +27,10 @@ const (
 	// years.  However, if the field is interpreted as a timestamp, given
 	// the lock time is a uint32, the max is sometime around 2106.
 	lockTimeThreshold uint32 = 5e8 // Tue Nov 5 00:53:20 1985 UTC
+
+	// maxRejectReasonLen is the maximum length of a sanitized reject reason
+	// that will be logged.
+	maxRejectReasonLen = 200
 )
 
 // Loggers per subsytem.  Note that backendLog is a seelog logger that all of
@@ -250,6 +255,30 @@ func locatorSummary(locator []*btcwire.ShaHash, stopHash *btcwire.ShaHash) strin
 
 }
 
+// sanitizeString strips any characters which are even remotely dangerous, such
+// as html control characters, from the passed string.  It also limits it to
+// the passed maximum size, which can be 0 for unlimited.  When the string is
+// limited, it will also add "..." to the string to indicate it was truncated.
+func sanitizeString(str string, maxLength uint) string {
+	const safeChars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXY" +
+		"Z01234567890 .,;_/:?@"
+
+	// Strip any characters not in the safeChars string removed.
+	str = strings.Map(func(r rune) rune {
+		if strings.IndexRune(safeChars, r) >= 0 {
+			return r
+		}
+		return -1
+	}, str)
+
+	// Limit the string to the max allowed length.
+	if maxLength > 0 && uint(len(str)) > maxLength {
+		str = str[:maxLength]
+		str = str + "..."
+	}
+	return str
+}
+
 // messageSummary returns a human-readable string which summarizes a message.
 // Not all messages have or need a summary.  This is used for debug logging.
 func messageSummary(msg btcwire.Message) string {
@@ -308,6 +337,20 @@ func messageSummary(msg btcwire.Message) string {
 
 	case *btcwire.MsgHeaders:
 		return fmt.Sprintf("num %d", len(msg.Headers))
+
+	case *btcwire.MsgReject:
+		// Ensure the variable length strings don't contain any
+		// characters which are even remotely dangerous such as HTML
+		// control characters, etc.  Also limit them to sane length for
+		// logging.
+		rejCommand := sanitizeString(msg.Cmd, btcwire.CommandSize)
+		rejReason := sanitizeString(msg.Reason, maxRejectReasonLen)
+		summary := fmt.Sprintf("cmd %v, code %v, reason %v", rejCommand,
+			msg.Code, rejReason)
+		if rejCommand == btcwire.CmdBlock || rejCommand == btcwire.CmdTx {
+			summary += fmt.Sprintf(", hash %v", msg.Hash)
+		}
+		return summary
 	}
 
 	// No summary for other messages.

mempool.go

@@ -20,17 +20,6 @@ import (
 	"github.com/conformal/btcwire"
 )
 
-// TxRuleError identifies a rule violation.  It is used to indicate that
-// processing of a transaction failed due to one of the many validation
-// rules.  The caller can use type assertions to determine if a failure was
-// specifically due to a rule violation.
-type TxRuleError string
-
-// Error satisfies the error interface to print human-readable errors.
-func (e TxRuleError) Error() string {
-	return string(e)
-}
-
 const (
 	// mempoolHeight is the height used for the "block" height field of the
 	// contextual transaction information provided in a transaction store.
@@ -182,36 +171,41 @@ func checkPkScriptStandard(pkScript []byte, scriptClass btcscript.ScriptClass) e
 	case btcscript.MultiSigTy:
 		numPubKeys, numSigs, err := btcscript.CalcMultiSigStats(pkScript)
 		if err != nil {
-			return err
+			str := fmt.Sprintf("multi-signature script parse "+
+				"failure: %v", err)
+			return txRuleError(btcwire.RejectNonstandard, str)
 		}
 
 		// A standard multi-signature public key script must contain
 		// from 1 to maxStandardMultiSigKeys public keys.
 		if numPubKeys < 1 {
-			return TxRuleError("multi-signature script with no pubkeys")
+			str := "multi-signature script with no pubkeys"
+			return txRuleError(btcwire.RejectNonstandard, str)
 		}
 		if numPubKeys > maxStandardMultiSigKeys {
 			str := fmt.Sprintf("multi-signature script with %d "+
 				"public keys which is more than the allowed "+
 				"max of %d", numPubKeys, maxStandardMultiSigKeys)
-			return TxRuleError(str)
+			return txRuleError(btcwire.RejectNonstandard, str)
 		}
 
 		// A standard multi-signature public key script must have at
 		// least 1 signature and no more signatures than available
 		// public keys.
 		if numSigs < 1 {
-			return TxRuleError("multi-signature script with no signatures")
+			return txRuleError(btcwire.RejectNonstandard,
+				"multi-signature script with no signatures")
 		}
 		if numSigs > numPubKeys {
 			str := fmt.Sprintf("multi-signature script with %d "+
 				"signatures which is more than the available "+
 				"%d public keys", numSigs, numPubKeys)
-			return TxRuleError(str)
+			return txRuleError(btcwire.RejectNonstandard, str)
 		}
 
 	case btcscript.NonStandardTy:
-		return TxRuleError("non-standard script form")
+		return txRuleError(btcwire.RejectNonstandard,
+			"non-standard script form")
 	}
 
 	return nil
@@ -232,13 +226,14 @@ func checkTransactionStandard(tx *btcutil.Tx, height int64) error {
 		str := fmt.Sprintf("transaction version %d is not in the "+
 			"valid range of %d-%d", msgTx.Version, 1,
 			btcwire.TxVersion)
-		return TxRuleError(str)
+		return txRuleError(btcwire.RejectNonstandard, str)
 	}
 
 	// The transaction must be finalized to be standard and therefore
 	// considered for inclusion in a block.
 	if !btcchain.IsFinalizedTransaction(tx, height, time.Now()) {
-		return TxRuleError("transaction is not finalized")
+		return txRuleError(btcwire.RejectNonstandard,
+			"transaction is not finalized")
 	}
 
 	// Since extremely large transactions with a lot of inputs can cost
@@ -249,7 +244,7 @@ func checkTransactionStandard(tx *btcutil.Tx, height int64) error {
 	if serializedLen > maxStandardTxSize {
 		str := fmt.Sprintf("transaction size of %v is larger than max "+
 			"allowed size of %v", serializedLen, maxStandardTxSize)
-		return TxRuleError(str)
+		return txRuleError(btcwire.RejectNonstandard, str)
 	}
 
 	for i, txIn := range msgTx.TxIn {
@@ -262,7 +257,7 @@ func checkTransactionStandard(tx *btcutil.Tx, height int64) error {
 				"script size of %d bytes is large than max "+
 				"allowed size of %d bytes", i, sigScriptLen,
 				maxStandardSigScriptSize)
-			return TxRuleError(str)
+			return txRuleError(btcwire.RejectNonstandard, str)
 		}
 
 		// Each transaction input signature script must only contain
@@ -270,7 +265,7 @@ func checkTransactionStandard(tx *btcutil.Tx, height int64) error {
 		if !btcscript.IsPushOnlyScript(txIn.SignatureScript) {
 			str := fmt.Sprintf("transaction input %d: signature "+
 				"script is not push only", i)
-			return TxRuleError(str)
+			return txRuleError(btcwire.RejectNonstandard, str)
 		}
 
 		// Each transaction input signature script must only contain
@@ -280,7 +275,7 @@ func checkTransactionStandard(tx *btcutil.Tx, height int64) error {
 		if !btcscript.HasCanonicalPushes(txIn.SignatureScript) {
 			str := fmt.Sprintf("transaction input %d: signature "+
 				"script has a non-canonical data push", i)
-			return TxRuleError(str)
+			return txRuleError(btcwire.RejectNonstandard, str)
 		}
 	}
 
@@ -291,8 +286,15 @@ func checkTransactionStandard(tx *btcutil.Tx, height int64) error {
 		scriptClass := btcscript.GetScriptClass(txOut.PkScript)
 		err := checkPkScriptStandard(txOut.PkScript, scriptClass)
 		if err != nil {
+			// Attempt to extract a reject code from the error so
+			// it can be retained.  When not possible, fall back to
+			// a non standard error.
+			rejectCode, found := extractRejectCode(err)
+			if !found {
+				rejectCode = btcwire.RejectNonstandard
+			}
 			str := fmt.Sprintf("transaction output %d: %v", i, err)
-			return TxRuleError(str)
+			return txRuleError(rejectCode, str)
 		}
 
 		// Accumulate the number of outputs which only carry data.
@@ -303,15 +305,15 @@ func checkTransactionStandard(tx *btcutil.Tx, height int64) error {
 		if isDust(txOut) {
 			str := fmt.Sprintf("transaction output %d: payment "+
 				"of %d is dust", i, txOut.Value)
-			return TxRuleError(str)
+			return txRuleError(btcwire.RejectDust, str)
 		}
 	}
 
 	// A standard transaction must not have more than one output script that
 	// only carries data.
 	if numNullDataOutputs > 1 {
-		return TxRuleError("more than one transaction output is a " +
+		str := "more than one transaction output in a nulldata script"
-			"nulldata script")
+		return txRuleError(btcwire.RejectNonstandard, str)
 	}
 
 	return nil
@@ -341,7 +343,9 @@ func checkInputsStandard(tx *btcutil.Tx, txStore btcchain.TxStore) error {
 		scriptInfo, err := btcscript.CalcScriptInfo(txIn.SignatureScript,
 			originPkScript, true)
 		if err != nil {
-			return err
+			str := fmt.Sprintf("transaction input #%d script parse "+
+				"failure: %v", i, err)
+			return txRuleError(btcwire.RejectNonstandard, str)
 		}
 
 		// A negative value for expected inputs indicates the script is
@@ -349,7 +353,7 @@ func checkInputsStandard(tx *btcutil.Tx, txStore btcchain.TxStore) error {
 		if scriptInfo.ExpectedInputs < 0 {
 			str := fmt.Sprintf("transaction input #%d expects %d "+
 				"inputs", i, scriptInfo.ExpectedInputs)
-			return TxRuleError(str)
+			return txRuleError(btcwire.RejectNonstandard, str)
 		}
 
 		// The script pair is non-standard if the number of available
@@ -359,7 +363,7 @@ func checkInputsStandard(tx *btcutil.Tx, txStore btcchain.TxStore) error {
 				"inputs, but referenced output script provides "+
 				"%d", i, scriptInfo.ExpectedInputs,
 				scriptInfo.NumInputs)
-			return TxRuleError(str)
+			return txRuleError(btcwire.RejectNonstandard, str)
 		}
 	}
 
@@ -511,7 +515,7 @@ func (mp *txMemPool) maybeAddOrphan(tx *btcutil.Tx) error {
 		str := fmt.Sprintf("orphan transaction size of %d bytes is "+
 			"larger than max allowed size of %d bytes",
 			serializedLen, maxOrphanTxSize)
-		return TxRuleError(str)
+		return txRuleError(btcwire.RejectNonstandard, str)
 	}
 
 	// Add the orphan if the none of the above disqualified it.
@@ -677,7 +681,7 @@ func (mp *txMemPool) checkPoolDoubleSpend(tx *btcutil.Tx) error {
 		if txR, exists := mp.outpoints[txIn.PreviousOutpoint]; exists {
 			str := fmt.Sprintf("transaction %v in the pool "+
 				"already spends the same coins", txR.Sha())
-			return TxRuleError(str)
+			return txRuleError(btcwire.RejectDuplicate, str)
 		}
 	}
 
@@ -744,7 +748,7 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe
 	// be a quick check to weed out duplicates.
 	if mp.haveTransaction(txHash) {
 		str := fmt.Sprintf("already have transaction %v", txHash)
-		return TxRuleError(str)
+		return txRuleError(btcwire.RejectDuplicate, str)
 	}
 
 	// Perform preliminary sanity checks on the transaction.  This makes
@@ -752,8 +756,8 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe
 	// transactions are allowed into blocks.
 	err := btcchain.CheckTransactionSanity(tx)
 	if err != nil {
-		if _, ok := err.(btcchain.RuleError); ok {
+		if cerr, ok := err.(btcchain.RuleError); ok {
-			return TxRuleError(err.Error())
+			return chainRuleError(cerr)
 		}
 		return err
 	}
@@ -762,7 +766,7 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe
 	if btcchain.IsCoinBase(tx) {
 		str := fmt.Sprintf("transaction %v is an individual coinbase",
 			txHash)
-		return TxRuleError(str)
+		return txRuleError(btcwire.RejectInvalid, str)
 	}
 
 	// Don't accept transactions with a lock time after the maximum int32
@@ -772,7 +776,7 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe
 	if tx.MsgTx().LockTime > math.MaxInt32 {
 		str := fmt.Sprintf("transaction %v has a lock time after "+
 			"2038 which is not accepted yet", txHash)
-		return TxRuleError(str)
+		return txRuleError(btcwire.RejectNonstandard, str)
 	}
 
 	// Get the current height of the main chain.  A standalone transaction
@@ -780,6 +784,8 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe
 	// one more than the current height.
 	_, curHeight, err := mp.server.db.NewestSha()
 	if err != nil {
+		// This is an unexpected error so don't turn it into a rule
+		// error.
 		return err
 	}
 	nextBlockHeight := curHeight + 1
@@ -789,9 +795,16 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe
 	if !activeNetParams.RelayNonStdTxs {
 		err := checkTransactionStandard(tx, nextBlockHeight)
 		if err != nil {
-			str := fmt.Sprintf("transaction %v is not a standard "+
+			// Attempt to extract a reject code from the error so
-				"transaction: %v", txHash, err)
+			// it can be retained.  When not possible, fall back to
-			return TxRuleError(str)
+			// a non standard error.
+			rejectCode, found := extractRejectCode(err)
+			if !found {
+				rejectCode = btcwire.RejectNonstandard
+			}
+			str := fmt.Sprintf("transaction %v is not standard: %v",
+				txHash, err)
+			return txRuleError(rejectCode, str)
 		}
 	}
 
@@ -814,6 +827,9 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe
 	// needing to do a separate lookup.
 	txStore, err := mp.fetchInputTransactions(tx)
 	if err != nil {
+		if cerr, ok := err.(btcchain.RuleError); ok {
+			return chainRuleError(cerr)
+		}
 		return err
 	}
 
@@ -822,7 +838,8 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe
 	if txD, exists := txStore[*txHash]; exists && txD.Err == nil {
 		for _, isOutputSpent := range txD.Spent {
 			if !isOutputSpent {
-				return TxRuleError("transaction already exists")
+				return txRuleError(btcwire.RejectDuplicate,
+					"transaction already exists")
 			}
 		}
 	}
@@ -844,8 +861,8 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe
 	// used later.
 	txFee, err := btcchain.CheckTransactionInputs(tx, nextBlockHeight, txStore)
 	if err != nil {
-		if _, ok := err.(btcchain.RuleError); ok {
+		if cerr, ok := err.(btcchain.RuleError); ok {
-			return TxRuleError(err.Error())
+			return chainRuleError(cerr)
 		}
 		return err
 	}
@@ -855,9 +872,16 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe
 	if !activeNetParams.RelayNonStdTxs {
 		err := checkInputsStandard(tx, txStore)
 		if err != nil {
+			// Attempt to extract a reject code from the error so
+			// it can be retained.  When not possible, fall back to
+			// a non standard error.
+			rejectCode, found := extractRejectCode(err)
+			if !found {
+				rejectCode = btcwire.RejectNonstandard
+			}
 			str := fmt.Sprintf("transaction %v has a non-standard "+
 				"input: %v", txHash, err)
-			return TxRuleError(str)
+			return txRuleError(rejectCode, str)
 		}
 	}
 
@@ -871,7 +895,7 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe
 		str := fmt.Sprintf("transaction %v has %d fees which is under "+
 			"the required amount of %d", txHash, txFee,
 			minRequiredFee)
-		return TxRuleError(str)
+		return txRuleError(btcwire.RejectInsufficientFee, str)
 	}
 
 	// Free-to-relay transactions are rate limited here to prevent
@@ -888,7 +912,7 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe
 		if mp.pennyTotal >= cfg.FreeTxRelayLimit*10*1000 {
 			str := fmt.Sprintf("transaction %v has 0 fees and has "+
 				"been rejected by the rate limiter", txHash)
-			return TxRuleError(str)
+			return txRuleError(btcwire.RejectInsufficientFee, str)
 		}
 		oldTotal := mp.pennyTotal
 
@@ -903,6 +927,9 @@ func (mp *txMemPool) maybeAcceptTransaction(tx *btcutil.Tx, isOrphan *bool, isNe
 	err = btcchain.ValidateTransactionScripts(tx, txStore,
 		standardScriptVerifyFlags)
 	if err != nil {
+		if cerr, ok := err.(btcchain.RuleError); ok {
+			return chainRuleError(cerr)
+		}
 		return err
 	}
 
@@ -1036,7 +1063,14 @@ func (mp *txMemPool) ProcessTransaction(tx *btcutil.Tx, allowOrphan, rateLimit b
 		// The transaction is an orphan (has inputs missing).  Reject
 		// it if the flag to allow orphans is not set.
 		if !allowOrphan {
-			return TxRuleError("transaction spends unknown inputs")
+			// NOTE: RejectDuplicate is really not an accurate
+			// reject code here, but it matches the reference
+			// implementation and there isn't a better choice due
+			// to the limited number of reject codes.  Missing
+			// inputs is assumed to mean they are already spent
+			// which is not really always the case.
+			return txRuleError(btcwire.RejectDuplicate,
+				"transaction spends unknown inputs")
 		}
 
 		// Potentially add the orphan transaction to the orphan pool.

mempoolerror.go

@@ -0,0 +1,134 @@
+// Copyright (c) 2014 Conformal Systems LLC.
+// Use of this source code is governed by an ISC
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+	"github.com/conformal/btcchain"
+	"github.com/conformal/btcwire"
+)
+
+// RuleError identifies a rule violation.  It is used to indicate that
+// processing of a transaction failed due to one of the many validation
+// rules.  The caller can use type assertions to determine if a failure was
+// specifically due to a rule violation and use the Err field to access the
+// underlying error, which will be either a TxRuleError or a btcchain.RuleError.
+type RuleError struct {
+	Err error
+}
+
+// Error satisfies the error interface and prints human-readable errors.
+func (e RuleError) Error() string {
+	if e.Err == nil {
+		return "<nil>"
+	}
+	return e.Err.Error()
+}
+
+// TxRuleError identifies a rule violation.  It is used to indicate that
+// processing of a transaction failed due to one of the many validation
+// rules.  The caller can use type assertions to determine if a failure was
+// specifically due to a rule violation and access the ErrorCode field to
+// ascertain the specific reason for the rule violation.
+type TxRuleError struct {
+	RejectCode  btcwire.RejectCode // The code to send with reject messages
+	Description string             // Human readable description of the issue
+}
+
+// Error satisfies the error interface and prints human-readable errors.
+func (e TxRuleError) Error() string {
+	return e.Description
+}
+
+// txRuleError creates an underlying TxRuleError with the given a set of
+// arguments and returns a RuleError that encapsulates it.
+func txRuleError(c btcwire.RejectCode, desc string) RuleError {
+	return RuleError{
+		Err: TxRuleError{RejectCode: c, Description: desc},
+	}
+}
+
+// chainRuleError returns a RuleError that encapsulates the given
+// btcchain.RuleError.
+func chainRuleError(chainErr btcchain.RuleError) RuleError {
+	return RuleError{
+		Err: chainErr,
+	}
+}
+
+// extractRejectCode attempts to return a relevant reject code for a given error
+// by examining the error for known types.  It will return true if a code
+// was successfully extracted.
+func extractRejectCode(err error) (btcwire.RejectCode, bool) {
+	// Pull the underlying error out of a RuleError.
+	if rerr, ok := err.(RuleError); ok {
+		err = rerr.Err
+	}
+
+	switch err := err.(type) {
+	case btcchain.RuleError:
+		// Convert the chain error to a reject code.
+		var code btcwire.RejectCode
+		switch err.ErrorCode {
+		// Rejected due to duplicate.
+		case btcchain.ErrDuplicateBlock:
+			fallthrough
+		case btcchain.ErrDoubleSpend:
+			code = btcwire.RejectDuplicate
+
+		// Rejected due to obsolete version.
+		case btcchain.ErrBlockVersionTooOld:
+			code = btcwire.RejectObsolete
+
+		// Rejected due to checkpoint.
+		case btcchain.ErrCheckpointTimeTooOld:
+			fallthrough
+		case btcchain.ErrDifficultyTooLow:
+			fallthrough
+		case btcchain.ErrBadCheckpoint:
+			fallthrough
+		case btcchain.ErrForkTooOld:
+			code = btcwire.RejectCheckpoint
+
+		// Everything else is due to the block or transaction being invalid.
+		default:
+			code = btcwire.RejectInvalid
+		}
+
+		return code, true
+
+	case TxRuleError:
+		return err.RejectCode, true
+
+	case nil:
+		return btcwire.RejectInvalid, false
+	}
+
+	return btcwire.RejectInvalid, false
+}
+
+// errToRejectErr examines the underlying type of the error and returns a reject
+// code and string appropriate to be sent in a btcwire.MsgReject message.
+func errToRejectErr(err error) (btcwire.RejectCode, string) {
+	// Return the reject code along with the error text if it can be
+	// extracted from the error.
+	rejectCode, found := extractRejectCode(err)
+	if found {
+		return rejectCode, err.Error()
+	}
+
+	// Return a generic rejected string if there is no error.  This really
+	// should not happen unless the code elsewhere is not setting an error
+	// as it should be, but it's best to be safe and simply return a generic
+	// string rather than allowing the following code that derferences the
+	// err to panic.
+	if err == nil {
+		return btcwire.RejectInvalid, "rejected"
+	}
+
+	// When the underlying error is not one of the above cases, just return
+	// btcwire.RejectInvalid with a generic rejected string plus the error
+	// text.
+	return btcwire.RejectInvalid, "rejected: " + err.Error()
+}

peer.go

@@ -8,6 +8,7 @@ import (
 	"bytes"
 	"container/list"
 	"fmt"
+	"io"
 	"net"
 	"strconv"
 	"sync"
@@ -26,7 +27,7 @@ import (
 
 const (
 	// maxProtocolVersion is the max protocol version the peer supports.
-	maxProtocolVersion = 70001
+	maxProtocolVersion = 70002
 
 	// outputBufferSize is the number of elements the output channels use.
 	outputBufferSize = 50
@@ -353,12 +354,35 @@ func (p *peer) handleVersionMsg(msg *btcwire.MsgVersion) {
 		return
 	}
 
-	p.StatsMtx.Lock() // Updating a bunch of stats.
+	// Notify and disconnect clients that have a protocol version that is
+	// too old.
+	if msg.ProtocolVersion < int32(btcwire.MultipleAddressVersion) {
+		// Send a reject message indicating the protocol version is
+		// obsolete and wait for the message to be sent before
+		// disconnecting.
+		reason := fmt.Sprintf("protocol version must be %d or greater",
+			btcwire.MultipleAddressVersion)
+		p.PushRejectMsg(msg.Command(), btcwire.RejectObsolete, reason,
+			nil, true)
+		p.Disconnect()
+		return
+	}
+
+	// Updating a bunch of stats.
+	p.StatsMtx.Lock()
+
 	// Limit to one version message per peer.
 	if p.versionKnown {
 		p.logError("Only one version message per peer is allowed %s.",
 			p)
 		p.StatsMtx.Unlock()
+
+		// Send an reject message indicating the version message was
+		// incorrectly sent twice and wait for the message to be sent
+		// before disconnecting.
+		p.PushRejectMsg(msg.Command(), btcwire.RejectDuplicate,
+			"duplicate version message", nil, true)
+
 		p.Disconnect()
 		return
 	}
@@ -653,6 +677,40 @@ func (p *peer) PushGetHeadersMsg(locator btcchain.BlockLocator, stopHash *btcwir
 	return nil
 }
 
+// PushRejectMsg sends a reject message for the provided command, reject code,
+// and reject reason, and hash.  The hash will only be used when the command
+// is a tx or block and should be nil in other cases.  The wait parameter will
+// cause the function to block until the reject message has actually been sent.
+func (p *peer) PushRejectMsg(command string, code btcwire.RejectCode, reason string, hash *btcwire.ShaHash, wait bool) {
+	// Don't bother sending the reject message if the protocol version
+	// is too low.
+	if p.VersionKnown() && p.ProtocolVersion() < btcwire.RejectVersion {
+		return
+	}
+
+	msg := btcwire.NewMsgReject(command, code, reason)
+	if command == btcwire.CmdTx || command == btcwire.CmdBlock {
+		if hash == nil {
+			peerLog.Warnf("Sending a reject message for command "+
+				"type %v which should have specified a hash "+
+				"but does not", command)
+			hash = &zeroHash
+		}
+		msg.Hash = *hash
+	}
+
+	// Send the message without waiting if the caller has not requested it.
+	if !wait {
+		p.QueueMessage(msg, nil)
+		return
+	}
+
+	// Send the message and block until it has been sent before returning.
+	doneChan := make(chan struct{}, 1)
+	p.QueueMessage(msg, doneChan)
+	<-doneChan
+}
+
 // handleMemPoolMsg is invoked when a peer receives a mempool bitcoin message.
 // It creates and sends an inventory message with the contents of the memory
 // pool up to the maximum inventory allowed per message.  When the peer has a
@@ -1231,9 +1289,11 @@ func (p *peer) writeMessage(msg btcwire.Message) {
 		switch msg.(type) {
 		case *btcwire.MsgVersion:
 			// This is OK.
+		case *btcwire.MsgReject:
+			// This is OK.
 		default:
-			// We drop all messages other than version if we
+			// Drop all messages other than version and reject if
-			// haven't done the handshake already.
+			// the handshake has not already been done.
 			return
 		}
 	}
@@ -1332,10 +1392,32 @@ out:
 				continue
 			}
 
-			// Only log the error if we're not forcibly disconnecting.
+			// Only log the error and possibly send reject message
+			// if we're not forcibly disconnecting.
 			if atomic.LoadInt32(&p.disconnect) == 0 {
-				p.logError("Can't read message from %s: %v",
+				errMsg := fmt.Sprintf("Can't read message "+
-					p, err)
+					"from %s: %v", p, err)
+				p.logError(errMsg)
+
+				// Only send the reject message if it's not
+				// because the remote client disconnected.
+				if err != io.EOF {
+					// Push a reject message for the
+					// malformed message and wait for the
+					// message to be sent before
+					// disconnecting.
+					//
+					// NOTE: Ideally this would include the
+					// command in the header if at least
+					// that much of the message was valid,
+					// but that is not currently exposed by
+					// btcwire, so just used malformed for
+					// the command.
+					p.PushRejectMsg("malformed",
+						btcwire.RejectMalformed, errMsg,
+						nil, true)
+				}
+
 			}
 			break out
 		}
@@ -1344,8 +1426,14 @@ out:
 		p.StatsMtx.Unlock()
 
 		// Ensure version message comes first.
-		if _, ok := rmsg.(*btcwire.MsgVersion); !ok && !p.VersionKnown() {
+		if vmsg, ok := rmsg.(*btcwire.MsgVersion); !ok && !p.VersionKnown() {
-			p.logError("A version message must precede all others")
+			errStr := "A version message must precede all others"
+			p.logError(errStr)
+
+			// Push a reject message and wait for the message to be
+			// sent before disconnecting.
+			p.PushRejectMsg(vmsg.Command(), btcwire.RejectMalformed,
+				errStr, nil, true)
 			break out
 		}
 
@@ -1417,6 +1505,10 @@ out:
 		case *btcwire.MsgFilterLoad:
 			p.handleFilterLoadMsg(msg)
 
+		case *btcwire.MsgReject:
+			// Nothing to do currently.  Logging of the rejected
+			// message is handled already in readMessage.
+
 		default:
 			peerLog.Debugf("Received unhandled message of type %v: Fix Me",
 				rmsg.Command())

rpcserver.go

@@ -2868,7 +2868,7 @@ func handleSendRawTransaction(s *rpcServer, cmd btcjson.Cmd, closeChan <-chan st
 		// so log it as an actual error.  In both cases, a JSON-RPC
 		// error is returned to the client with the deserialization
 		// error code (to match bitcoind behavior).
-		if _, ok := err.(TxRuleError); ok {
+		if _, ok := err.(RuleError); ok {
 			rpcsLog.Debugf("Rejected transaction %v: %v", tx.Sha(),
 				err)
 		} else {