Merge 02dd534672 into a50af6e4c4

doc/release-notes-29060.md

@@ -0,0 +1,12 @@
+- Logging and RPC
+
+  - Bitcoin Core now provide the specific reason and index of the first
+    non-standard input in a transaction, applicable to non-standard inputs
+    received via P2P and also changes the responses of transaction sending
+    RPC's (`submitpackage`, and `sendrawtransaction`) and `testmempoolaccept` RPC.
+
+  - `bad-txns-nonstandard-inputs` will message will now be:
+    - `bad-txns-input-script-unknown`: When the input’s `scriptPubKey` is non-standard or, if standard, has an incorrect witness program size.
+    - `bad-txns-input-p2sh-scriptsig-malformed`: When the input’s `scriptPubKey` is P2SH, but the `scriptSig` is invalid.
+    - `bad-txns-input-scriptcheck-missing`: When the input’s `scriptPubKey` is P2SH, but `scriptSig` is missing.
+    - `bad-txns-input-p2sh-redeemscript-sigops`: When the input’s `scriptPubKey` is P2SH, but `scriptSig` has too many signature operations that exceeds  `MAX_P2SH_SIGOPS`.

src/bench/ccoins_caching.cpp

@@ -49,8 +49,7 @@ static void CCoinsCaching(benchmark::Bench& bench)
     // Benchmark.
     const CTransaction tx_1(t1);
     bench.run([&] {
-        bool success{AreInputsStandard(tx_1, coins)};
-        assert(success);
+        assert(HasNonStandardInput(tx_1, coins).IsValid());
     });
 }
 

src/policy/policy.cpp

@@ -18,6 +18,7 @@
 #include <script/solver.h>
 #include <serialize.h>
 #include <span.h>
+#include <tinyformat.h>
 
 #include <algorithm>
 #include <cstddef>
@@ -186,10 +187,11 @@ bool IsStandardTx(const CTransaction& tx, const std::optional<unsigned>& max_dat
  *
  * Note that only the non-witness portion of the transaction is checked here.
  */
-bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs)
+TxValidationState HasNonStandardInput(const CTransaction& tx, const CCoinsViewCache& mapInputs)
 {
+    TxValidationState state;
     if (tx.IsCoinBase()) {
-        return true; // Coinbases don't use vin normally
+        return state; // Coinbases don't use vin normally
     }
 
     for (unsigned int i = 0; i < tx.vin.size(); i++) {
@@ -197,27 +199,35 @@ bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs)
 
         std::vector<std::vector<unsigned char> > vSolutions;
         TxoutType whichType = Solver(prev.scriptPubKey, vSolutions);
-        if (whichType == TxoutType::NONSTANDARD || whichType == TxoutType::WITNESS_UNKNOWN) {
+        if (whichType == TxoutType::NONSTANDARD) {
+            state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txns-input-script-unknown", strprintf("input %u", i));
+        } else if (whichType == TxoutType::WITNESS_UNKNOWN) {
             // WITNESS_UNKNOWN failures are typically also caught with a policy
             // flag in the script interpreter, but it can be helpful to catch
             // this type of NONSTANDARD transaction earlier in transaction
             // validation.
-            return false;
+            state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txns-input-witness-unknown", strprintf("input %u", i));
         } else if (whichType == TxoutType::SCRIPTHASH) {
             std::vector<std::vector<unsigned char> > stack;
+            ScriptError serror;
             // convert the scriptSig into a stack, so we can inspect the redeemScript
-            if (!EvalScript(stack, tx.vin[i].scriptSig, SCRIPT_VERIFY_NONE, BaseSignatureChecker(), SigVersion::BASE))
-                return false;
-            if (stack.empty())
-                return false;
+            if (!EvalScript(stack, tx.vin[i].scriptSig, SCRIPT_VERIFY_NONE, BaseSignatureChecker(), SigVersion::BASE, &serror)) {
+                state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txns-input-p2sh-scriptsig-malformed", strprintf("input %u: %s", i, ScriptErrorString(serror)));
+                return state;
+            }
+            if (stack.empty()) {
+                state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txns-input-scriptcheck-missing", strprintf("input %u", i));
+                return state;
+            }
             CScript subscript(stack.back().begin(), stack.back().end());
-            if (subscript.GetSigOpCount(true) > MAX_P2SH_SIGOPS) {
-                return false;
+            unsigned int sigop_count = subscript.GetSigOpCount(true);
+            if (sigop_count > MAX_P2SH_SIGOPS) {
+                state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txns-input-p2sh-redeemscript-sigops", strprintf("input %u: %u > %u", i, sigop_count, MAX_P2SH_SIGOPS));
             }
         }
     }
 
-    return true;
+    return state;
 }
 
 bool IsWitnessStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs)

src/policy/policy.h

@@ -8,6 +8,7 @@
 
 #include <consensus/amount.h>
 #include <consensus/consensus.h>
+#include <consensus/validation.h>
 #include <primitives/transaction.h>
 #include <script/interpreter.h>
 #include <script/solver.h>
@@ -152,11 +153,12 @@ static constexpr decltype(CTransaction::version) TX_MAX_STANDARD_VERSION{3};
 */
 bool IsStandardTx(const CTransaction& tx, const std::optional<unsigned>& max_datacarrier_bytes, bool permit_bare_multisig, const CFeeRate& dust_relay_fee, std::string& reason);
 /**
-* Check for standard transaction types
-* @param[in] mapInputs       Map of previous transactions that have outputs we're spending
-* @return True if all inputs (scriptSigs) use only standard transaction forms
-*/
-bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs);
+ * Check for standard transaction types
+ * @param[in] mapInputs       Map of previous transactions that have outputs we're spending
+ * @returns valid TxValidationState if all inputs (scriptSigs) use only standard transaction forms else returns
+ * invalid TxValidationState which states why the first invalid input is not standard
+ */
+TxValidationState HasNonStandardInput(const CTransaction& tx, const CCoinsViewCache& mapInputs);
 /**
 * Check if the transaction is over standard P2WSH resources limit:
 * 3600bytes witnessScript size, 80bytes per witness stack element, 100 witness stack elements

src/test/fuzz/coins_view.cpp

@@ -239,7 +239,7 @@ FUZZ_TARGET(coins_view, .init = initialize_coins_view)
                 assert(expected_code_path);
             },
             [&] {
-                (void)AreInputsStandard(CTransaction{random_mutable_transaction}, coins_view_cache);
+                (void)HasNonStandardInput(CTransaction{random_mutable_transaction}, coins_view_cache);
             },
             [&] {
                 TxValidationState state;

src/test/fuzz/transaction.cpp

@@ -89,7 +89,7 @@ FUZZ_TARGET(transaction, .init = initialize_transaction)
 
     CCoinsView coins_view;
     const CCoinsViewCache coins_view_cache(&coins_view);
-    (void)AreInputsStandard(tx, coins_view_cache);
+    (void)HasNonStandardInput(tx, coins_view_cache);
     (void)IsWitnessStandard(tx, coins_view_cache);
 
     if (tx.GetTotalSize() < 250'000) { // Avoid high memory usage (with msan) due to json encoding

src/test/script_p2sh_tests.cpp

@@ -275,7 +275,7 @@ BOOST_AUTO_TEST_CASE(switchover)
     BOOST_CHECK_MESSAGE(err == SCRIPT_ERR_EQUALVERIFY, ScriptErrorString(err));
 }
 
-BOOST_AUTO_TEST_CASE(AreInputsStandard)
+BOOST_AUTO_TEST_CASE(HasNonStandardInput)
 {
     CCoinsView coinsDummy;
     CCoinsViewCache coins(&coinsDummy);
@@ -292,7 +292,7 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard)
         keys.push_back(key[i].GetPubKey());
 
     CMutableTransaction txFrom;
-    txFrom.vout.resize(7);
+    txFrom.vout.resize(10);
 
     // First three are standard:
     CScript pay1 = GetScriptForDestination(PKHash(key[0].GetPubKey()));
@@ -334,7 +334,24 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard)
     CScript twentySigops; twentySigops << OP_CHECKMULTISIG;
     BOOST_CHECK(keystore.AddCScript(twentySigops));
     txFrom.vout[6].scriptPubKey = GetScriptForDestination(ScriptHash(twentySigops));
-    txFrom.vout[6].nValue = 6000;
+    txFrom.vout[6].nValue = 3000;
+
+    // vout[7] is non-standard because it lacks sigops, therefore failing Solver
+    CScript no_sigops;
+    txFrom.vout[7].scriptPubKey = no_sigops;
+    txFrom.vout[7].nValue = 1000;
+
+    // vout [8] is non-standard because it contains OP_RETURN in its scriptSig.
+    static const unsigned char op_return[] = {OP_RETURN};
+    const auto op_return_script = CScript(op_return, op_return + sizeof(op_return));
+    txFrom.vout[8].scriptPubKey = GetScriptForDestination(ScriptHash(op_return_script));
+    txFrom.vout[8].nValue = 1000;
+
+    // vout[9] is non-standard because its witness is unknown
+    CScript witnessUnknown;
+    witnessUnknown << OP_16 << ToByteVector(uint256::ONE);
+    txFrom.vout[9].scriptPubKey = witnessUnknown;
+    txFrom.vout[9].nValue = 1000;
 
     AddCoins(coins, CTransaction(txFrom), 0);
 
@@ -360,10 +377,11 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard)
     txTo.vin[3].scriptSig << OP_11 << OP_11 << std::vector<unsigned char>(oneAndTwo.begin(), oneAndTwo.end());
     txTo.vin[4].scriptSig << std::vector<unsigned char>(fifteenSigops.begin(), fifteenSigops.end());
 
-    BOOST_CHECK(::AreInputsStandard(CTransaction(txTo), coins));
+    BOOST_CHECK(::HasNonStandardInput(CTransaction(txTo), coins).IsValid());
     // 22 P2SH sigops for all inputs (1 for vin[0], 6 for vin[3], 15 for vin[4]
     BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txTo), coins), 22U);
 
+    // TxoutType::SCRIPTHASH
     CMutableTransaction txToNonStd1;
     txToNonStd1.vout.resize(1);
     txToNonStd1.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key[1].GetPubKey()));
@@ -373,7 +391,11 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard)
     txToNonStd1.vin[0].prevout.hash = txFrom.GetHash();
     txToNonStd1.vin[0].scriptSig << std::vector<unsigned char>(sixteenSigops.begin(), sixteenSigops.end());
 
-    BOOST_CHECK(!::AreInputsStandard(CTransaction(txToNonStd1), coins));
+    const auto txToNonStd1_res = ::HasNonStandardInput(CTransaction(txToNonStd1), coins);
+    BOOST_CHECK(txToNonStd1_res.IsInvalid());
+    BOOST_CHECK_EQUAL(txToNonStd1_res.GetRejectReason(), "bad-txns-input-p2sh-redeemscript-sigops");
+    BOOST_CHECK_EQUAL(txToNonStd1_res.GetDebugMessage(), "input 0: 16 > 15");
+
     BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd1), coins), 16U);
 
     CMutableTransaction txToNonStd2;
@@ -385,8 +407,79 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard)
     txToNonStd2.vin[0].prevout.hash = txFrom.GetHash();
     txToNonStd2.vin[0].scriptSig << std::vector<unsigned char>(twentySigops.begin(), twentySigops.end());
 
-    BOOST_CHECK(!::AreInputsStandard(CTransaction(txToNonStd2), coins));
+    std::vector<std::vector<unsigned char>> vSolutions;
+    BOOST_CHECK_EQUAL(Solver(txFrom.vout[6].scriptPubKey, vSolutions), TxoutType::SCRIPTHASH);
+    const auto txToNonStd2_res = ::HasNonStandardInput(CTransaction(txToNonStd2), coins);
+    BOOST_CHECK(txToNonStd2_res.IsInvalid());
+    BOOST_CHECK_EQUAL(txToNonStd2_res.GetRejectReason(), "bad-txns-input-p2sh-redeemscript-sigops");
+    BOOST_CHECK_EQUAL(txToNonStd2_res.GetDebugMessage(), "input 0: 20 > 15");
     BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd2), coins), 20U);
+
+    CMutableTransaction txToNonStd2_no_scriptSig;
+    txToNonStd2_no_scriptSig.vout.resize(1);
+    txToNonStd2_no_scriptSig.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key[1].GetPubKey()));
+    txToNonStd2_no_scriptSig.vout[0].nValue = 1000;
+    txToNonStd2_no_scriptSig.vin.resize(1);
+    txToNonStd2_no_scriptSig.vin[0].prevout.n = 6;
+    txToNonStd2_no_scriptSig.vin[0].prevout.hash = txFrom.GetHash();
+
+    BOOST_CHECK_EQUAL(Solver(txFrom.vout[6].scriptPubKey, vSolutions), TxoutType::SCRIPTHASH);
+    const auto txToNonStd2_no_scriptSig_res = ::HasNonStandardInput(CTransaction(txToNonStd2_no_scriptSig), coins);
+    BOOST_CHECK(txToNonStd2_no_scriptSig_res.IsInvalid());
+    BOOST_CHECK_EQUAL(txToNonStd2_no_scriptSig_res.GetRejectReason(), "bad-txns-input-scriptcheck-missing");
+    BOOST_CHECK_EQUAL(txToNonStd2_no_scriptSig_res.GetDebugMessage(), "input 0");
+    BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd2), coins), 20U);
+
+    // TxoutType::NONSTANDARD
+    CMutableTransaction txToNonStd3;
+    txToNonStd3.vout.resize(1);
+    txToNonStd3.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key[1].GetPubKey()));
+    txToNonStd3.vout[0].nValue = 1000;
+    txToNonStd3.vin.resize(1);
+    txToNonStd3.vin[0].prevout.n = 7;
+    txToNonStd3.vin[0].prevout.hash = txFrom.GetHash();
+
+    BOOST_CHECK_EQUAL(Solver(txFrom.vout[7].scriptPubKey, vSolutions), TxoutType::NONSTANDARD);
+    const auto txToNonStd3_res = ::HasNonStandardInput(CTransaction(txToNonStd3), coins);
+    BOOST_CHECK(txToNonStd3_res.IsInvalid());
+    BOOST_CHECK_EQUAL(txToNonStd3_res.GetRejectReason(), "bad-txns-input-script-unknown");
+    BOOST_CHECK_EQUAL(txToNonStd3_res.GetDebugMessage(), "input 0");
+
+    // TxoutType::INCORRECT_SCRIPTSIG
+    CMutableTransaction txToNonStd4;
+    txToNonStd4.vout.resize(1);
+    txToNonStd4.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key[1].GetPubKey()));
+    txToNonStd4.vout[0].nValue = 1000;
+    txToNonStd4.vin.resize(1);
+    txToNonStd4.vin[0].prevout.n = 8;
+    txToNonStd4.vin[0].prevout.hash = txFrom.GetHash();
+    txToNonStd4.vin[0].scriptSig = op_return_script;
+
+    // out args for EvalScript
+    std::vector<std::vector<unsigned char>> stack;
+    ScriptError serror;
+
+    BOOST_CHECK_EQUAL(Solver(txFrom.vout[8].scriptPubKey, vSolutions), TxoutType::SCRIPTHASH);
+    BOOST_CHECK(!EvalScript(stack, txToNonStd4.vin[0].scriptSig, SCRIPT_VERIFY_NONE, BaseSignatureChecker(), SigVersion::BASE, &serror));
+    BOOST_CHECK_EQUAL(serror, SCRIPT_ERR_OP_RETURN);
+    const auto txToNonStd4_res = ::HasNonStandardInput(CTransaction(txToNonStd4), coins);
+    BOOST_CHECK(txToNonStd4_res.IsInvalid());
+    BOOST_CHECK_EQUAL(txToNonStd4_res.GetRejectReason(), "bad-txns-input-p2sh-scriptsig-malformed");
+    BOOST_CHECK_EQUAL(txToNonStd4_res.GetDebugMessage(), "input 0: OP_RETURN was encountered");
+
+    // TxoutType::WITNESS_UNKNOWN
+    CMutableTransaction txWitnessUnknown;
+    txWitnessUnknown.vout.resize(1);
+    txWitnessUnknown.vout[0].scriptPubKey = GetScriptForDestination(PKHash(key[1].GetPubKey()));
+    txWitnessUnknown.vout[0].nValue = 1000;
+    txWitnessUnknown.vin.resize(1);
+    txWitnessUnknown.vin[0].prevout.n = 9;
+    txWitnessUnknown.vin[0].prevout.hash = txFrom.GetHash();
+    BOOST_CHECK_EQUAL(Solver(txFrom.vout[9].scriptPubKey, vSolutions), TxoutType::WITNESS_UNKNOWN);
+    const auto txWitnessUnknown_res = ::HasNonStandardInput(CTransaction(txWitnessUnknown), coins);
+    BOOST_CHECK(txWitnessUnknown_res.IsInvalid());
+    BOOST_CHECK_EQUAL(txWitnessUnknown_res.GetRejectReason(), "bad-txns-input-witness-unknown");
+    BOOST_CHECK_EQUAL(txWitnessUnknown_res.GetDebugMessage(), "input 0");
 }
 
 BOOST_AUTO_TEST_SUITE_END()

src/test/transaction_tests.cpp

@@ -445,7 +445,7 @@ BOOST_AUTO_TEST_CASE(test_Get)
     t1.vout[0].nValue = 90*CENT;
     t1.vout[0].scriptPubKey << OP_1;
 
-    BOOST_CHECK(AreInputsStandard(CTransaction(t1), coins));
+    BOOST_CHECK(HasNonStandardInput(CTransaction(t1), coins).IsValid());
 }
 
 static void CreateCreditAndSpend(const FillableSigningProvider& keystore, const CScript& outscript, CTransactionRef& output, CMutableTransaction& input, bool success = true)

src/validation.cpp

@@ -878,8 +878,11 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
         return false; // state filled in by CheckTxInputs
     }
 
-    if (m_pool.m_opts.require_standard && !AreInputsStandard(tx, m_view)) {
-        return state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txns-nonstandard-inputs");
+    if (m_pool.m_opts.require_standard) {
+        state = HasNonStandardInput(tx, m_view);
+        if (state.IsInvalid()) {
+            return false;
+        }
     }
 
     // Check for non-standard witnesses.

test/functional/p2p_segwit.py

@@ -1385,7 +1385,7 @@ class SegWitTest(BitcoinTestFramework):
         # First we test this transaction against std_node
         # making sure the txid is added to the reject filter
         self.std_node.announce_tx_and_wait_for_getdata(tx3)
-        test_transaction_acceptance(self.nodes[1], self.std_node, tx3, with_witness=True, accepted=False, reason="bad-txns-nonstandard-inputs")
+        test_transaction_acceptance(self.nodes[1], self.std_node, tx3, with_witness=True, accepted=False, reason="bad-txns-input-witness-unknown")
         # Now the node will no longer ask for getdata of this transaction when advertised by same txid
         self.std_node.announce_tx_and_wait_for_getdata(tx3, success=False)