Merge #19681: 0.19: Add txids with non-standard inputs to reject filter

52c3bec1bad4fc602a3911b44de48c6b0dbdfd25 test addition of unknown segwit spends to txid reject filter (Gregory Sanders)
2ea826cfc46ee8edfca059d0fd95ebe03122f9f2 Add txids with non-standard inputs to reject filter (Suhas Daftuar)

Pull request description:

  Backport of #19620 to 0.19.

ACKs for top commit:
  instagibbs:
    utACK 52c3bec1ba
  jnewbery:
    utACK 52c3bec1ba
  jonasschnelli:
    utACK 52c3bec1bad4fc602a3911b44de48c6b0dbdfd25

Tree-SHA512: 76b52d3fb0f9d88674dd186dee611bf0a2349b17549ef7909b4b37ace5b64d4edce56d71410e7b743e7e7d18855b84ff4b555a5edac26f67786abb9a264fa256
This commit is contained in:
Jonas Schnelli 2020-08-28 09:08:31 +02:00
commit aee9d2306a
No known key found for this signature in database
GPG Key ID: 1EB776BB03C7922D
5 changed files with 36 additions and 8 deletions

View File

@ -48,7 +48,8 @@ enum class ValidationInvalidReason {
BLOCK_TIME_FUTURE, //!< block timestamp was > 2 hours in the future (or our clock is bad)
BLOCK_CHECKPOINT, //!< the block failed to meet one of our checkpoints
// Only loose txn:
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, //!< a transaction was missing some of its inputs
TX_PREMATURE_SPEND, //!< transaction spends a coinbase too early, or violates locktime/sequence locks
/**
@ -72,6 +73,7 @@ inline bool IsTransactionReason(ValidationInvalidReason r)
return r == ValidationInvalidReason::NONE ||
r == ValidationInvalidReason::CONSENSUS ||
r == ValidationInvalidReason::RECENT_CONSENSUS_CHANGE ||
r == ValidationInvalidReason::TX_INPUTS_NOT_STANDARD ||
r == ValidationInvalidReason::TX_NOT_STANDARD ||
r == ValidationInvalidReason::TX_PREMATURE_SPEND ||
r == ValidationInvalidReason::TX_MISSING_INPUTS ||

View File

@ -1055,6 +1055,7 @@ static bool MaybePunishNode(NodeId nodeid, const CValidationState& state, bool v
return true;
case ValidationInvalidReason::RECENT_CONSENSUS_CHANGE:
case ValidationInvalidReason::BLOCK_TIME_FUTURE:
case ValidationInvalidReason::TX_INPUTS_NOT_STANDARD:
case ValidationInvalidReason::TX_NOT_STANDARD:
case ValidationInvalidReason::TX_MISSING_INPUTS:
case ValidationInvalidReason::TX_PREMATURE_SPEND:
@ -1846,10 +1847,15 @@ void static ProcessOrphanTx(CConnman* connman, std::set<uint256>& orphan_work_se
// Probably non-standard or insufficient fee
LogPrint(BCLog::MEMPOOL, " removed orphan tx %s\n", orphanHash.ToString());
assert(IsTransactionReason(orphan_state.GetReason()));
if (!orphanTx.HasWitness() && orphan_state.GetReason() != ValidationInvalidReason::TX_WITNESS_MUTATED) {
if ((!orphanTx.HasWitness() && orphan_state.GetReason() != ValidationInvalidReason::TX_WITNESS_MUTATED) ||
orphan_state.GetReason() == ValidationInvalidReason::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);
}
@ -2574,10 +2580,15 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
}
} else {
assert(IsTransactionReason(state.GetReason()));
if (!tx.HasWitness() && state.GetReason() != ValidationInvalidReason::TX_WITNESS_MUTATED) {
if ((!tx.HasWitness() && state.GetReason() != ValidationInvalidReason::TX_WITNESS_MUTATED) ||
state.GetReason() == ValidationInvalidReason::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

@ -678,8 +678,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(ValidationInvalidReason::TX_NOT_STANDARD, false, REJECT_NONSTANDARD, "bad-txns-nonstandard-inputs");
if (fRequireStandard && !AreInputsStandard(tx, m_view)) {
return state.Invalid(ValidationInvalidReason::TX_INPUTS_NOT_STANDARD, false, REJECT_NONSTANDARD, "bad-txns-nonstandard-inputs");
}
// Check for non-standard witness in P2WSH
if (tx.HasWitness() && fRequireStandard && !IsWitnessStandard(tx, m_view))

View File

@ -1391,7 +1391,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:
@ -1399,8 +1399,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")