diff options
author | Suhas Daftuar <sdaftuar@gmail.com> | 2020-02-07 04:30:41 -0500 |
---|---|---|
committer | John Newbery <john@johnnewbery.com> | 2020-09-24 13:24:10 +0100 |
commit | 879a3cf2c2367d51310204d21030f3b218582c30 (patch) | |
tree | c6576fd5335346d022dacd86d27dbf56eaeab7ae | |
parent | c1d6a1003d601ec4ff7d9507563254b29868182f (diff) | |
download | bitcoin-879a3cf2c2367d51310204d21030f3b218582c30.tar.xz |
Make TX_WITNESS_STRIPPED its own rejection reason
Previously, TX_WITNESS_MUTATED could be returned during transaction validation
for either transactions that had a witness that was non-standard, or for
transactions that had no witness but were invalid due to segwit validation
rules.
However, for txid/wtxid-relay considerations, net_processing distinguishes the
witness stripped case separately, because it affects whether a wtxid should be
able to be added to the reject filter. It is safe to add the wtxid of a
witness-mutated transaction to the filter (as that wtxid shouldn't collide with
the txid, and hence it wouldn't interfere with transaction relay from
txid-relay peers), but it is not safe to add the wtxid (== txid) of a
witness-stripped transaction to the filter, because that would interfere with
relay of another transaction with the same txid (but different wtxid) when
relaying from txid-relay peers.
Also updates the comment explaining this logic, and explaining that we can get
rid of this complexity once there's a sufficient deployment of wtxid-relaying
peers on the network.
-rw-r--r-- | src/consensus/validation.h | 6 | ||||
-rw-r--r-- | src/net_processing.cpp | 5 | ||||
-rw-r--r-- | src/validation.cpp | 2 |
3 files changed, 9 insertions, 4 deletions
diff --git a/src/consensus/validation.h b/src/consensus/validation.h index 3a90cd69b3..f7d476951b 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -31,12 +31,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 0527bd2022..8893397e83 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1092,6 +1092,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; @@ -1934,7 +1935,7 @@ 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) { + if (orphan_state.GetResult() != TxValidationResult::TX_WITNESS_STRIPPED) { // We can add the wtxid of this transaction to our reject filter. // Do not add txids of witness transactions or witness-stripped // transactions to the filter, as they can have been malleated; @@ -2708,7 +2709,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec recentRejects->insert(tx.GetWitnessHash()); } } else { - if (tx.HasWitness() || state.GetResult() != TxValidationResult::TX_WITNESS_MUTATED) { + if (state.GetResult() != TxValidationResult::TX_WITNESS_STRIPPED) { // We can add the wtxid of this transaction to our reject filter. // Do not add txids of witness transactions or witness-stripped // transactions to the filter, as they can have been malleated; diff --git a/src/validation.cpp b/src/validation.cpp index 5a98b2cb92..a4adc81b5a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -916,7 +916,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 |