From 50f880f1eff18c6730c442538ff742218b56493f Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Tue, 5 Dec 2023 14:41:02 +0800 Subject: [PATCH] multi: map `btcd` mempool acceptance errors to bitcoind's `testmempoolaccept` 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`. --- blockchain/validate.go | 10 +-- mempool/mempool.go | 21 +++--- mempool/policy.go | 12 +-- rpcclient/errors.go | 165 ++++++++++++++++++++++++++++++++++++++++- rpcserver.go | 5 +- 5 files changed, 190 insertions(+), 23 deletions(-) diff --git a/blockchain/validate.go b/blockchain/validate.go index 438f455428b..dc1190a4bab 100644 --- 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 index 6913ac9e78b..db04f619a5f 100644 --- 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/policy.go b/mempool/policy.go index 758f7e06a9c..862767d0c89 100644 --- 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 index 62de9b3806b..78f34bef2ee 100644 --- 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 index 36c00be18f9..2433286ac70 100644 --- 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)