diff options
| -rw-r--r-- | src/consensus/validation.h | 6 | ||||
| -rw-r--r-- | src/net_processing.cpp | 35 | ||||
| -rw-r--r-- | src/validation.cpp | 2 |
3 files changed, 33 insertions, 10 deletions
diff --git a/src/consensus/validation.h b/src/consensus/validation.h index a79e7b9d12..337ee244d3 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -30,12 +30,16 @@ enum class TxValidationResult { TX_MISSING_INPUTS, //!< transaction was missing some of its inputs TX_PREMATURE_SPEND, //!< transaction spends a coinbase too early, or violates locktime/sequence locks /** - * Transaction might be missing a witness, have a witness prior to SegWit + * Transaction might have a witness prior to SegWit * activation, or witness may have been malleated (which includes * non-standard witnesses). */ TX_WITNESS_MUTATED, /** + * Transaction is missing a witness. + */ + TX_WITNESS_STRIPPED, + /** * Tx already in mempool or conflicts with a tx in the chain * (if it conflicts with another tx in mempool, we use MEMPOOL_POLICY as it failed to reach the RBF threshold) * Currently this is only used if the transaction already exists in the mempool or on chain. diff --git a/src/net_processing.cpp b/src/net_processing.cpp index bc09e94094..d2ac86ea49 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1160,6 +1160,7 @@ static bool MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state, case TxValidationResult::TX_MISSING_INPUTS: case TxValidationResult::TX_PREMATURE_SPEND: case TxValidationResult::TX_WITNESS_MUTATED: + case TxValidationResult::TX_WITNESS_STRIPPED: case TxValidationResult::TX_CONFLICT: case TxValidationResult::TX_MEMPOOL_POLICY: break; @@ -2031,10 +2032,19 @@ void static ProcessOrphanTx(CConnman& connman, CTxMemPool& mempool, std::set<uin // Has inputs but not accepted to mempool // Probably non-standard or insufficient fee LogPrint(BCLog::MEMPOOL, " removed orphan tx %s\n", orphanHash.ToString()); - if (orphanTx.HasWitness() || orphan_state.GetResult() != TxValidationResult::TX_WITNESS_MUTATED) { - // Do not use rejection cache for witness transactions or - // witness-stripped transactions, as they can have been malleated. - // See https://github.com/bitcoin/bitcoin/issues/8279 for details. + if (orphan_state.GetResult() != TxValidationResult::TX_WITNESS_STRIPPED) { + // Do not add txids of witness transactions or witness-stripped + // transactions to the filter, as they can have been malleated; + // adding such txids to the reject filter would potentially + // interfere with relay of valid transactions from peers that + // do not support wtxid-based relay. See + // https://github.com/bitcoin/bitcoin/issues/8279 for details. + // We can remove this restriction (and always add wtxids to + // the filter even for witness stripped transactions) once + // wtxid-based relay is broadly deployed. + // See also comments in https://github.com/bitcoin/bitcoin/pull/18044#discussion_r443419034 + // for concerns around weakening security of unupgraded nodes + // if we start doing this too early. assert(recentRejects); recentRejects->insert(orphanTx.GetWitnessHash()); } @@ -2992,10 +3002,19 @@ void ProcessMessage( recentRejects->insert(tx.GetWitnessHash()); } } else { - if (tx.HasWitness() || state.GetResult() != TxValidationResult::TX_WITNESS_MUTATED) { - // Do not use rejection cache for witness transactions or - // witness-stripped transactions, as they can have been malleated. - // See https://github.com/bitcoin/bitcoin/issues/8279 for details. + if (state.GetResult() != TxValidationResult::TX_WITNESS_STRIPPED) { + // Do not add txids of witness transactions or witness-stripped + // transactions to the filter, as they can have been malleated; + // adding such txids to the reject filter would potentially + // interfere with relay of valid transactions from peers that + // do not support wtxid-based relay. See + // https://github.com/bitcoin/bitcoin/issues/8279 for details. + // We can remove this restriction (and always add wtxids to + // the filter even for witness stripped transactions) once + // wtxid-based relay is broadly deployed. + // See also comments in https://github.com/bitcoin/bitcoin/pull/18044#discussion_r443419034 + // for concerns around weakening security of unupgraded nodes + // if we start doing this too early. assert(recentRejects); recentRejects->insert(tx.GetWitnessHash()); if (RecursiveDynamicUsage(*ptx) < 100000) { diff --git a/src/validation.cpp b/src/validation.cpp index d271be6310..ff7345b55f 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -939,7 +939,7 @@ bool MemPoolAccept::PolicyScriptChecks(ATMPArgs& args, Workspace& ws, Precompute if (!tx.HasWitness() && CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, txdata) && !CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, txdata)) { // Only the witness is missing, so the transaction itself may be fine. - state.Invalid(TxValidationResult::TX_WITNESS_MUTATED, + state.Invalid(TxValidationResult::TX_WITNESS_STRIPPED, state.GetRejectReason(), state.GetDebugMessage()); } return false; // state filled in by CheckInputScripts |