Merge #19680: 0.20: Add txids with non-standard inputs to reject filter

107cf1515e test addition of unknown segwit spends to txid reject filter (Gregory Sanders)
06f9c5c3be Add txids with non-standard inputs to reject filter (Suhas Daftuar)

Pull request description:

  Backport of #19620 to 0.20.

ACKs for top commit:
  instagibbs:
    utACK 107cf1515e
  fjahr:
    utACK 107cf1515e
  jnewbery:
    utACK 107cf1515e

Tree-SHA512: d89c75fefdb6b50f40adfcf3cfdb2a791122e4d6a5903cb553c664042963577beb97a73319f9d1cb8320d32846778233c95255812c37fb4c7b1b25da161a6595
This commit is contained in:
fanquake
2020-09-04 19:53:55 +08:00
5 changed files with 35 additions and 8 deletions

View File

@@ -26,7 +26,8 @@ enum class TxValidationResult {
* is uninteresting.
*/
TX_RECENT_CONSENSUS_CHANGE,
TX_NOT_STANDARD, //!< didn't meet our local policy rules
TX_INPUTS_NOT_STANDARD, //!< inputs (covered by txid) failed policy rules
TX_NOT_STANDARD, //!< otherwise didn't meet our local policy rules
TX_MISSING_INPUTS, //!< transaction was missing some of its inputs
TX_PREMATURE_SPEND, //!< transaction spends a coinbase too early, or violates locktime/sequence locks
/**

View File

@@ -1069,6 +1069,7 @@ static bool MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state,
}
// Conflicting (but not necessarily invalid) data or different policy:
case TxValidationResult::TX_RECENT_CONSENSUS_CHANGE:
case TxValidationResult::TX_INPUTS_NOT_STANDARD:
case TxValidationResult::TX_NOT_STANDARD:
case TxValidationResult::TX_MISSING_INPUTS:
case TxValidationResult::TX_PREMATURE_SPEND:
@@ -1901,10 +1902,15 @@ 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 ((!orphanTx.HasWitness() && orphan_state.GetResult() != TxValidationResult::TX_WITNESS_MUTATED) ||
orphan_state.GetResult() == TxValidationResult::TX_INPUTS_NOT_STANDARD) {
// 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.
// However, if the transaction failed for TX_INPUTS_NOT_STANDARD,
// then we know that the witness was irrelevant to the policy
// failure, since this check depends only on the txid
// (the scriptPubKey being spent is covered by the txid).
assert(recentRejects);
recentRejects->insert(orphanHash);
}
@@ -2596,10 +2602,15 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
recentRejects->insert(tx.GetHash());
}
} else {
if (!tx.HasWitness() && state.GetResult() != TxValidationResult::TX_WITNESS_MUTATED) {
if ((!tx.HasWitness() && state.GetResult() != TxValidationResult::TX_WITNESS_MUTATED) ||
state.GetResult() == TxValidationResult::TX_INPUTS_NOT_STANDARD) {
// 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.
// However, if the transaction failed for TX_INPUTS_NOT_STANDARD,
// then we know that the witness was irrelevant to the policy
// failure, since this check depends only on the txid
// (the scriptPubKey being spent is covered by the txid).
assert(recentRejects);
recentRejects->insert(tx.GetHash());
if (RecursiveDynamicUsage(*ptx) < 100000) {

View File

@@ -152,6 +152,8 @@ bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, const CFeeR
* script can be anything; an attacker could use a very
* expensive-to-check-upon-redemption script like:
* DUP CHECKSIG DROP ... repeated 100 times... OP_1
*
* Note that only the non-witness portion of the transaction is checked here.
*/
bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs)
{
@@ -164,7 +166,11 @@ bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs)
std::vector<std::vector<unsigned char> > vSolutions;
txnouttype whichType = Solver(prev.scriptPubKey, vSolutions);
if (whichType == TX_NONSTANDARD) {
if (whichType == TX_NONSTANDARD || whichType == TX_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;
} else if (whichType == TX_SCRIPTHASH) {
std::vector<std::vector<unsigned char> > stack;

View File

@@ -665,8 +665,9 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
}
// Check for non-standard pay-to-script-hash in inputs
if (fRequireStandard && !AreInputsStandard(tx, m_view))
return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "bad-txns-nonstandard-inputs");
if (fRequireStandard && !AreInputsStandard(tx, m_view)) {
return state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txns-nonstandard-inputs");
}
// Check for non-standard witness in P2WSH
if (tx.HasWitness() && fRequireStandard && !IsWitnessStandard(tx, m_view))

View File

@@ -1393,7 +1393,7 @@ class SegWitTest(BitcoinTestFramework):
temp_utxo.pop() # last entry in temp_utxo was the output we just spent
temp_utxo.append(UTXO(tx2.sha256, 0, tx2.vout[0].nValue))
# Spend everything in temp_utxo back to an OP_TRUE output.
# Spend everything in temp_utxo into an segwit v1 output.
tx3 = CTransaction()
total_value = 0
for i in temp_utxo:
@@ -1401,8 +1401,16 @@ class SegWitTest(BitcoinTestFramework):
tx3.wit.vtxinwit.append(CTxInWitness())
total_value += i.nValue
tx3.wit.vtxinwit[-1].scriptWitness.stack = [witness_program]
tx3.vout.append(CTxOut(total_value - 1000, CScript([OP_TRUE])))
tx3.vout.append(CTxOut(total_value - 1000, script_pubkey))
tx3.rehash()
# First we test this transaction against fRequireStandard=true 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")
# 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, timeout=5, success=False)
# Spending a higher version witness output is not allowed by policy,
# even with fRequireStandard=false.
test_transaction_acceptance(self.nodes[0], self.test_node, tx3, with_witness=True, accepted=False, reason="reserved for soft-fork upgrades")