multi: map btcd mempool acceptance errors to bitcoind's `testmempoo…

…laccept` This commit creates a `RejectReasonMap` to map the errors returned from `btcd` to bitcoind's `testmempoolaccept` so the `RejectReason` is unified at the RPC level. To make sure the map keys are unique, the error strings are modified in `btcd`.
btcsuite · Roasbeef · Jan 16, 2024 · Jan 4, 2024 · Nov 3, 2023 · Nov 3, 2023
commit ef54c49df443815d50765e8c4f31a87944d950a6
diff --git a/blockchain/validate.go b/blockchain/validate.go
@@ -243,9 +243,9 @@ func CheckTransactionSanity(tx *btcutil.Tx) error {
 			return ruleError(ErrBadTxOutValue, str)
 		}
 		if satoshi > btcutil.MaxSatoshi {
-			str := fmt.Sprintf("transaction output value of %v is "+
-				"higher than max allowed value of %v", satoshi,
-				btcutil.MaxSatoshi)
+			str := fmt.Sprintf("transaction output value is "+
+				"higher than max allowed value: %v > %v ",
+				satoshi, btcutil.MaxSatoshi)
 			return ruleError(ErrBadTxOutValue, str)
 		}
 
@@ -968,8 +968,8 @@ func CheckTransactionInputs(tx *btcutil.Tx, txHeight int32, utxoView *UtxoViewpo
 			return 0, ruleError(ErrBadTxOutValue, str)
 		}
 		if originTxSatoshi > btcutil.MaxSatoshi {
-			str := fmt.Sprintf("transaction output value of %v is "+
-				"higher than max allowed value of %v",
+			str := fmt.Sprintf("transaction output value is "+
+				"higher than max allowed value: %v > %v ",
 				btcutil.Amount(originTxSatoshi),
 				btcutil.MaxSatoshi)
 			return 0, ruleError(ErrBadTxOutValue, str)

diff --git a/mempool/mempool.go b/mempool/mempool.go
@@ -592,9 +592,9 @@ func (mp *TxPool) checkPoolDoubleSpend(tx *btcutil.Tx) (bool, error) {
 		// transactions or if it doesn't signal replacement.
 		if mp.cfg.Policy.RejectReplacement ||
 			!mp.signalsReplacement(conflict, nil) {
-			str := fmt.Sprintf("output %v already spent by "+
-				"transaction %v in the memory pool",
-				txIn.PreviousOutPoint, conflict.Hash())
+			str := fmt.Sprintf("output already spent in mempool: "+
+				"output=%v, tx=%v", txIn.PreviousOutPoint,
+				conflict.Hash())
 			return false, txRuleError(wire.RejectDuplicate, str)
 		}
 
@@ -842,7 +842,7 @@ func (mp *TxPool) validateReplacement(tx *btcutil.Tx,
 	// exceed the maximum allowed.
 	conflicts := mp.txConflicts(tx)
 	if len(conflicts) > MaxReplacementEvictions {
-		str := fmt.Sprintf("replacement transaction %v evicts more "+
+		str := fmt.Sprintf("%v: replacement transaction evicts more "+
 			"transactions than permitted: max is %v, evicts %v",
 			tx.Hash(), MaxReplacementEvictions, len(conflicts))
 		return nil, txRuleError(wire.RejectNonstandard, str)
@@ -855,7 +855,7 @@ func (mp *TxPool) validateReplacement(tx *btcutil.Tx,
 		if _, ok := conflicts[ancestorHash]; !ok {
 			continue
 		}
-		str := fmt.Sprintf("replacement transaction %v spends parent "+
+		str := fmt.Sprintf("%v: replacement transaction spends parent "+
 			"transaction %v", tx.Hash(), ancestorHash)
 		return nil, txRuleError(wire.RejectInvalid, str)
 	}
@@ -876,7 +876,7 @@ func (mp *TxPool) validateReplacement(tx *btcutil.Tx,
 	)
 	for hash, conflict := range conflicts {
 		if txFeeRate <= mp.pool[hash].FeePerKB {
-			str := fmt.Sprintf("replacement transaction %v has an "+
+			str := fmt.Sprintf("%v: replacement transaction has an "+
 				"insufficient fee rate: needs more than %v, "+
 				"has %v", tx.Hash(), mp.pool[hash].FeePerKB,
 				txFeeRate)
@@ -897,7 +897,7 @@ func (mp *TxPool) validateReplacement(tx *btcutil.Tx,
 	// which is determined by our minimum relay fee.
 	minFee := calcMinRequiredTxRelayFee(txSize, mp.cfg.Policy.MinRelayTxFee)
 	if txFee < conflictsFee+minFee {
-		str := fmt.Sprintf("replacement transaction %v has an "+
+		str := fmt.Sprintf("%v: replacement transaction has an "+
 			"insufficient absolute fee: needs %v, has %v",
 			tx.Hash(), conflictsFee+minFee, txFee)
 		return nil, txRuleError(wire.RejectInsufficientFee, str)
@@ -1350,7 +1350,8 @@ func (mp *TxPool) checkMempoolAcceptance(tx *btcutil.Tx,
 	if mp.isTransactionInPool(txHash) || (rejectDupOrphans &&
 		mp.isOrphanInPool(txHash)) {
 
-		str := fmt.Sprintf("already have transaction %v", txHash)
+		str := fmt.Sprintf("already have transaction in mempool %v",
+			txHash)
 		return nil, txRuleError(wire.RejectDuplicate, str)
 	}
 
@@ -1368,7 +1369,7 @@ func (mp *TxPool) checkMempoolAcceptance(tx *btcutil.Tx,
 
 	// A standalone transaction must not be a coinbase transaction.
 	if blockchain.IsCoinBase(tx) {
-		str := fmt.Sprintf("transaction %v is an individual coinbase",
+		str := fmt.Sprintf("transaction is an individual coinbase %v",
 			txHash)
 
 		return nil, txRuleError(wire.RejectInvalid, str)
@@ -1418,7 +1419,7 @@ func (mp *TxPool) checkMempoolAcceptance(tx *btcutil.Tx,
 		entry := utxoView.LookupEntry(prevOut)
 		if entry != nil && !entry.IsSpent() {
 			return nil, txRuleError(wire.RejectDuplicate,
-				"transaction already exists")
+				"transaction already exists in blockchain")
 		}
 
 		utxoView.RemoveEntry(prevOut)

diff --git a/mempool/mempool_test.go b/mempool/mempool_test.go
@@ -1491,7 +1491,7 @@ func TestRBF(t *testing.T) {
 
 				return tx, nil
 			},
-			err: "already spent by transaction",
+			err: "already spent in mempool",
 		},
 		{
 			// A transaction cannot replace another if we don't
@@ -1522,7 +1522,7 @@ func TestRBF(t *testing.T) {
 
 				return tx, nil
 			},
-			err: "already spent by transaction",
+			err: "already spent in mempool",
 		},
 		{
 			// A transaction cannot replace another if doing so

diff --git a/mempool/policy.go b/mempool/policy.go
@@ -308,8 +308,8 @@ func CheckTransactionStandard(tx *btcutil.Tx, height int32,
 	// attacks.
 	txWeight := blockchain.GetTransactionWeight(tx)
 	if txWeight > maxStandardTxWeight {
-		str := fmt.Sprintf("weight of transaction %v is larger than max "+
-			"allowed weight of %v", txWeight, maxStandardTxWeight)
+		str := fmt.Sprintf("weight of transaction is larger than max "+
+			"allowed: %v > %v", txWeight, maxStandardTxWeight)
 		return txRuleError(wire.RejectNonstandard, str)
 	}
 
@@ -320,8 +320,8 @@ func CheckTransactionStandard(tx *btcutil.Tx, height int32,
 		sigScriptLen := len(txIn.SignatureScript)
 		if sigScriptLen > maxStandardSigScriptSize {
 			str := fmt.Sprintf("transaction input %d: signature "+
-				"script size of %d bytes is large than max "+
-				"allowed size of %d bytes", i, sigScriptLen,
+				"script size is larger than max allowed: "+
+				"%d > %d bytes", i, sigScriptLen,
 				maxStandardSigScriptSize)
 			return txRuleError(wire.RejectNonstandard, str)
 		}
@@ -359,8 +359,8 @@ func CheckTransactionStandard(tx *btcutil.Tx, height int32,
 		if scriptClass == txscript.NullDataTy {
 			numNullDataOutputs++
 		} else if IsDust(txOut, minRelayTxFee) {
-			str := fmt.Sprintf("transaction output %d: payment "+
-				"of %d is dust", i, txOut.Value)
+			str := fmt.Sprintf("transaction output %d: payment is "+
+				"dust: %v", i, txOut.Value)
 			return txRuleError(wire.RejectDust, str)
 		}
 	}

diff --git a/rpcclient/errors.go b/rpcclient/errors.go
@@ -1,6 +1,9 @@
 package rpcclient
 
-import "errors"
+import (
+	"errors"
+	"strings"
+)
 
 var (
 	// ErrBitcoindVersion is returned when running against a bitcoind that
@@ -10,4 +13,164 @@ var (
 	// ErrInvalidParam is returned when the caller provides an invalid
 	// parameter to an RPC method.
 	ErrInvalidParam = errors.New("invalid param")
+
+	// RejectReasonMap takes the error returned from
+	// `CheckMempoolAcceptance` in `btcd` and maps it to the reject reason
+	// that's returned from calling `testmempoolaccept` in `bitcoind`.
+	// references:
+	// - https://github.com/bitcoin/bitcoin/blob/master/test/functional/data/invalid_txs.py
+	// - https://github.com/bitcoin/bitcoin/blob/master/test/functional/mempool_accept.py
+	// - https://github.com/bitcoin/bitcoin/blob/master/src/validation.cpp
+	//
+	// Errors not mapped in `btcd`:
+	// - deployment error from `validateSegWitDeployment`.
+	// - the error when total inputs is higher than max allowed value from
+	//   `CheckTransactionInputs`.
+	// - the error when total outputs is higher than total inputs from
+	//   `CheckTransactionInputs`.
+	// - errors from `CalcSequenceLock`.
+	//
+	// NOTE: This is not an exhaustive list of errors, but it covers the
+	// usage case of LND.
+	//
+	//nolint:lll
+	RejectReasonMap = map[string]string{
+		// BIP125 related errors.
+		//
+		// When fee rate used or fees paid doesn't meet the
+		// requirements.
+		"replacement transaction has an insufficient fee rate":     "insufficient fee",
+		"replacement transaction has an insufficient absolute fee": "insufficient fee",
+
+		// When a transaction causes too many transactions being
+		// replaced. This is set by `MAX_REPLACEMENT_CANDIDATES` in
+		// `bitcoind` and defaults to 100.
+		"replacement transaction evicts more transactions than permitted": "too many potential replacements",
+
+		// When a transaction adds new unconfirmed inputs.
+		"replacement transaction spends new unconfirmed input": "replacement-adds-unconfirmed",
+
+		// A transaction that spends conflicting tx outputs that are
+		// rejected.
+		"replacement transaction spends parent transaction": "bad-txns-spends-conflicting-tx",
+
+		// A transaction that conflicts with an unconfirmed tx. Happens
+		// when RBF is not enabled.
+		"output already spent in mempool": "txn-mempool-conflict",
+
+		// A transaction with no outputs.
+		"transaction has no outputs": "bad-txns-vout-empty",
+
+		// A transaction with no inputs.
+		"transaction has no inputs": "bad-txns-vin-empty",
+
+		// A tiny transaction(in non-witness bytes) that is disallowed.
+		// TODO(yy): find/return this error in `btcd`.
+		// "": "tx-size-small",
+
+		// A transaction with duplicate inputs.
+		"transaction contains duplicate inputs": "bad-txns-inputs-duplicate",
+
+		// A non-coinbase transaction with coinbase-like outpoint.
+		"transaction input refers to previous output that is null": "bad-txns-prevout-null",
+
+		// A transaction pays too little fee.
+		"fees which is under the required amount":               "bad-txns-in-belowout",
+		"has insufficient priority":                             "bad-txns-in-belowout",
+		"has been rejected by the rate limiter due to low fees": "bad-txns-in-belowout",
+
+		// A transaction with negative output value.
+		"transaction output has negative value": "bad-txns-vout-negative",
+
+		// A transaction with too large output value.
+		"transaction output value is higher than max allowed value": "bad-txns-vout-toolarge",
+
+		// A transaction with too large sum of output values.
+		"total value of all transaction outputs exceeds max allowed value": "bad-txns-txouttotal-toolarge",
+
+		// TODO(yy): find/return this error in `btcd`.
+		// "": "mandatory-script-verify-flag-failed (Invalid OP_IF construction)",
+
+		// A transaction with too many sigops.
+		"sigop cost is too hight": "bad-txns-too-many-sigops",
+
+		// A transaction with invalid OP codes.
+		// TODO(yy): find/return this error in `btcd`.
+		// "": "disabled opcode",
+
+		// A transaction already in the blockchain.
+		"database contains entry for spent tx output": "txn-already-known",
+		"transaction already exists in blockchain":    "txn-already-known",
+
+		// A transaction in the mempool.
+		"already have transaction in mempool": "txn-already-in-mempool",
+
+		// A transaction with missing inputs, that never existed or
+		// only existed once in the past.
+		"either does not exist or has already been spent": "missing-inputs",
+
+		// A really large transaction.
+		"serialized transaction is too big": "bad-txns-oversize",
+
+		// A coinbase transaction.
+		"transaction is an invalid coinbase": "coinbase",
+
+		// Some nonstandard transactions - a version currently
+		// non-standard.
+		"transaction version": "version",
+
+		// Some nonstandard transactions - non-standard script.
+		"non-standard script form": "scriptpubkey",
+		"has a non-standard input": "scriptpubkey",
+
+		// Some nonstandard transactions - bare multisig script
+		// (2-of-3).
+		"milti-signature script": "bare-multisig",
+
+		// Some nonstandard transactions - not-pushonly scriptSig.
+		"signature script is not push only": "scriptsig-not-pushonly",
+
+		// Some nonstandard transactions - too large scriptSig (>1650
+		// bytes).
+		"signature script size is larger than max allowed": "scriptsig-size",
+
+		// Some nonstandard transactions - too large tx size.
+		"weight of transaction is larger than max allowed": "tx-size",
+
+		// Some nonstandard transactions - output too small.
+		"payment is dust": "dust",
+
+		// Some nonstandard transactions - muiltiple OP_RETURNs.
+		"more than one transaction output in a nulldata script": "multi-op-return",
+
+		// A timelocked transaction.
+		"transaction is not finalized":               "non-final",
+		"tried to spend coinbase transaction output": "non-final",
+
+		// A transaction that is locked by BIP68 sequence logic.
+		"transaction's sequence locks on inputs not met": "non-BIP68-final",
+
+		// Minimally-small transaction(in non-witness bytes) that is
+		// allowed.
+		// TODO(yy): find/return this error in `btcd`.
+		// "": "txn-same-nonwitness-data-in-mempools",
+	}
 )
+
+// MapBtcdErrToRejectReason takes an error returned from
+// `CheckMempoolAcceptance` and maps the error to a bitcoind reject reason.
+func MapBtcdErrToRejectReason(err error) string {
+	// Get the error string and turn it into lower case.
+	btcErr := strings.ToLower(err.Error())
+
+	// Iterate the map and find the matching error.
+	for keyErr, rejectReason := range RejectReasonMap {
+		// Match the substring.
+		if strings.Contains(btcErr, keyErr) {
+			return rejectReason
+		}
+	}
+
+	// If there's no match, return the error string directly.
+	return btcErr
+}
diff --git a/rpcserver.go b/rpcserver.go
@@ -39,6 +39,7 @@ import (
 	"github.com/btcsuite/btcd/mining"
 	"github.com/btcsuite/btcd/mining/cpuminer"
 	"github.com/btcsuite/btcd/peer"
+	"github.com/btcsuite/btcd/rpcclient"
 	"github.com/btcsuite/btcd/txscript"
 	"github.com/btcsuite/btcd/wire"
 	"github.com/btcsuite/websocket"
@@ -3856,7 +3857,9 @@ func handleTestMempoolAccept(s *rpcServer, cmd interface{},
 
 			// TODO(yy): differentiate the errors and put package
 			// error in `PackageError` field.
-			item.RejectReason = err.Error()
+			item.RejectReason = rpcclient.MapBtcdErrToRejectReason(
+				err,
+			)
 
 			results = append(results, item)