first commit wip

src/bench/ccoins_caching.cpp

@@ -44,8 +44,8 @@ static void CCoinsCaching(benchmark::Bench& bench)
     // Benchmark.
     const CTransaction tx_1(t1);
     bench.run([&] {
-        bool success{AreInputsStandard(tx_1, coins)};
-        assert(success);
+        auto result{AreInputsStandard(tx_1, coins)};
+        assert(!result.has_value());
     });
     ECC_Stop();
 }

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>
@@ -174,38 +175,50 @@ 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)
+std::optional<const NonStandardInputsReason> AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs)
 {
     if (tx.IsCoinBase()) {
-        return true; // Coinbases don't use vin normally
+        return std::nullopt; // Coinbases don't use vin normally
     }
 
     for (unsigned int i = 0; i < tx.vin.size(); i++) {
         const CTxOut& prev = mapInputs.AccessCoin(tx.vin[i].prevout).out;
 
         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) {
+            const auto result = NonStandardInputsReason{"bad-txns-input-script-nonstandard", strprintf("input %u", i)};
+            return result;
+        } 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;
+            const auto result = NonStandardInputsReason{"bad-txns-input-witness-unknown", strprintf("input %u", i)};
+            return result;
         } 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)) {
+                const auto result = NonStandardInputsReason{"bad-txns-input-scriptsig-failure", strprintf("input %u: %s", i, ScriptErrorString(serror))};
+                return result;
+            }
+            if (stack.empty()) {
+                const auto result = NonStandardInputsReason{"bad-txns-input-p2sh-no-redeemscript", strprintf("input %u", i)};
+                return result;
+            }
             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) {
+
+                const auto result = NonStandardInputsReason{"bad-txns-input-scriptcheck-sigops", strprintf("input %u: %u > %u", i, sigop_count, MAX_P2SH_SIGOPS)};
+                return result;
             }
         }
     }
 
-    return true;
+    return std::nullopt;
 }
 
 bool IsWitnessStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs)

src/policy/policy.h

@@ -121,6 +121,11 @@ static constexpr unsigned int STANDARD_NOT_MANDATORY_VERIFY_FLAGS{STANDARD_SCRIP
 /** Used as the flags parameter to sequence and nLocktime checks in non-consensus code. */
 static constexpr unsigned int STANDARD_LOCKTIME_VERIFY_FLAGS{LOCKTIME_VERIFY_SEQUENCE};
 
+struct NonStandardInputsReason {
+    std::string reason;
+    std::string debug;
+};
+
 CAmount GetDustThreshold(const CTxOut& txout, const CFeeRate& dustRelayFee);
 
 bool IsDust(const CTxOut& txout, const CFeeRate& dustRelayFee);
@@ -143,7 +148,7 @@ bool IsStandardTx(const CTransaction& tx, const std::optional<unsigned>& max_dat
 * @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);
+std::optional<const NonStandardInputsReason> AreInputsStandard(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/script_p2sh_tests.cpp

@@ -350,7 +350,7 @@ 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(!::AreInputsStandard(CTransaction(txTo), coins).has_value());
     // 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);
 
@@ -363,7 +363,7 @@ 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));
+    BOOST_CHECK(::AreInputsStandard(CTransaction(txToNonStd1), coins).has_value());
     BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd1), coins), 16U);
 
     CMutableTransaction txToNonStd2;
@@ -375,7 +375,7 @@ 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));
+    BOOST_CHECK(::AreInputsStandard(CTransaction(txToNonStd2), coins).has_value());
     BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd2), coins), 20U);
 }
 

src/test/transaction_tests.cpp

@@ -405,7 +405,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(!AreInputsStandard(CTransaction(t1), coins).has_value());
 }
 
 static void CreateCreditAndSpend(const FillableSigningProvider& keystore, const CScript& outscript, CTransactionRef& output, CMutableTransaction& input, bool success = true)

src/validation.cpp

@@ -822,8 +822,10 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
         return false; // state filled in by CheckTxInputs
     }
 
-    if (m_pool.m_require_standard && !AreInputsStandard(tx, m_view)) {
-        return state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txns-nonstandard-inputs");
+    const auto non_standardness_result = AreInputsStandard(tx, m_view);
+    if (m_pool.m_require_standard && non_standardness_result.has_value()) {
+
+        return state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, non_standardness_result.value().reason ,non_standardness_result.value().debug);
     }
 
     // Check for non-standard witnesses.

test/functional/p2p_segwit.py

@@ -1387,7 +1387,7 @@ def test_segwit_versions(self):
         # 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)