From 4b301666ff12c44b22ea568fdfd490810524f60a Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 12 Aug 2025 14:35:18 -0700 Subject: [PATCH] Merge bitcoin/bitcoin#33050: net, validation: don't punish peers for consensus-invalid txs 876dbdfb4702410dfd4037614dc9298a0c09c63e tests: drop expect_disconnect behaviour for tx relay (Anthony Towns) b29ae9efdfeeff774e32ee433ce67d8ed8ecd49f validation: only check input scripts once (Anthony Towns) 266dd0e10d08c0bfde63205db15d6c210a021b90 net_processing: drop MaybePunishNodeForTx (Anthony Towns) Pull request description: Because we do not discourage nodes for transactions we consider non-standard, we don't get any DoS protection from this check in adversarial scenarios, so remove the check entirely both to simplify the code and reduce the risk of splitting the network due to changes in tx relay policy. Then, because we no longer make use of the distinction between consensus and standardness failures during script validation, don't re-validate each script with only-consensus rules, reducing the cost to us of transactions that we won't relay. ACKs for top commit: achow101: ACK 876dbdfb4702410dfd4037614dc9298a0c09c63e darosior: re-ACK 876dbdfb4702410dfd4037614dc9298a0c09c63e sipa: re-ACK 876dbdfb4702410dfd4037614dc9298a0c09c63e glozow: ACK 876dbdfb4702410dfd4037614dc9298a0c09c63e Tree-SHA512: 8bb0395766dde54fc48f7077b80b88e35581aa6e3054d6d65735965147abefffa7348f0850bb3d46f6c2541fd384ecd40a00a57fa653adabff8a35582e2d1811 --- src/net_processing.cpp | 38 --------------------------- src/validation.cpp | 25 +++++++----------- test/functional/data/invalid_txs.py | 18 +------------ test/functional/feature_block.py | 5 +++- test/functional/feature_cltv.py | 15 ++++++----- test/functional/feature_dersig.py | 4 +-- test/functional/feature_nulldummy.py | 3 +-- test/functional/feature_segwit.py | 24 ++++++++--------- test/functional/p2p_invalid_tx.py | 6 ----- test/functional/p2p_segwit.py | 32 +++++++++++----------- test/functional/rpc_packages.py | 2 +- test/functional/test_framework/p2p.py | 8 ++---- 12 files changed, 57 insertions(+), 123 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b1e6e8bf85..4da940bc35 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -368,13 +368,6 @@ private: bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& state, bool via_compact_block, const std::string& message = ""); - /** - * Potentially disconnect and discourage a node based on the contents of a TxValidationState object - * - * @return Returns true if the peer was punished (probably disconnected) - */ - bool MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state, const std::string& message = ""); - /** Maybe disconnect a peer and discourage future connections from its address. * * @param[in] pnode The node to check. @@ -1437,34 +1430,6 @@ bool PeerManagerImpl::MaybePunishNodeForBlock(NodeId nodeid, const BlockValidati return false; } -bool PeerManagerImpl::MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state, const std::string& message) -{ - switch (state.GetResult()) { - case TxValidationResult::TX_RESULT_UNSET: - break; - // The node is providing invalid data: - case TxValidationResult::TX_CONSENSUS: - Misbehaving(nodeid, 100, message); - return true; - // 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: - case TxValidationResult::TX_WITNESS_MUTATED: - case TxValidationResult::TX_WITNESS_STRIPPED: - case TxValidationResult::TX_CONFLICT: - case TxValidationResult::TX_MEMPOOL_POLICY: - case TxValidationResult::TX_NO_MEMPOOL: - break; - } - if (message != "") { - LogPrint(BCLog::NET, "peer=%d: %s\n", nodeid, message); - } - return false; -} - bool PeerManagerImpl::BlockRequestAllowed(const CBlockIndex* pindex) { AssertLockHeld(cs_main); @@ -2392,8 +2357,6 @@ void PeerManagerImpl::ProcessOrphanTx(std::set& orphan_work_set) orphanHash.ToString(), from_peer, state.ToString()); - // Maybe punish peer that gave us an invalid orphan tx - MaybePunishNodeForTx(from_peer, state); } // Has inputs but not accepted to mempool // Probably non-standard or insufficient fee @@ -3559,7 +3522,6 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type, LogPrint(BCLog::MEMPOOLREJ, "%s from peer=%d was not accepted: %s\n", tx.GetHash().ToString(), pfrom.GetId(), state.ToString()); - MaybePunishNodeForTx(pfrom.GetId(), state); } return; } diff --git a/src/validation.cpp b/src/validation.cpp index a307c3036b..95feba0b77 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1793,21 +1793,15 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state, CCheck* check = new CScriptCheck(txdata.m_spent_outputs[i], tx, i, flags, cacheSigStore, &txdata); ScriptError serror = QueueCheck(pvChecks, check); if (serror != SCRIPT_ERR_OK) { + // Tx failures never trigger disconnections/bans. + // This is so that network splits aren't triggered + // either due to non-consensus relay policies (such as + // non-standard DER encodings or non-null dummy + // arguments) or due to new consensus rules introduced in + // soft forks. if (flags & STANDARD_NOT_MANDATORY_VERIFY_FLAGS) { - // Check whether the failure was caused by a - // non-mandatory script verification check, such as - // non-standard DER encodings or non-null dummy - // arguments; if so, ensure we return NOT_STANDARD - // instead of CONSENSUS to avoid downstream users - // splitting the network between upgraded and - // non-upgraded nodes by banning CONSENSUS-failing - // data providers. - CScriptCheck check2(txdata.m_spent_outputs[i], tx, i, - flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, cacheSigStore, &txdata); - if (check2()) { - return state.Invalid(TxValidationResult::TX_NOT_STANDARD, strprintf("non-mandatory-script-verify-flag (%s)", ScriptErrorString(serror))); - } - } + return state.Invalid(TxValidationResult::TX_NOT_STANDARD, strprintf("mempool-script-verify-flag-failed (%s)", ScriptErrorString(serror))); + } else { // MANDATORY flag failures correspond to // TxValidationResult::TX_CONSENSUS. Because CONSENSUS // failures are the most serious case of validation @@ -1817,7 +1811,8 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state, // support, to avoid splitting the network (but this // depends on the details of how net_processing handles // such errors). - return state.Invalid(TxValidationResult::TX_CONSENSUS, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(serror))); + return state.Invalid(TxValidationResult::TX_CONSENSUS, strprintf("mandatory-script-verify-flag-failed (%s)", ScriptErrorString(serror))); + } } } diff --git a/test/functional/data/invalid_txs.py b/test/functional/data/invalid_txs.py index fd9773c4c2..b4c0aaf61a 100644 --- a/test/functional/data/invalid_txs.py +++ b/test/functional/data/invalid_txs.py @@ -69,9 +69,6 @@ class BadTxTemplate: # Only specified if it differs from mempool acceptance error. block_reject_reason = "" - # Do we expect to be disconnected after submitting this tx? - expect_disconnect = False - # Is this tx considered valid when included in a block, but not for acceptance into # the mempool (i.e. does it violate policy but not consensus)? valid_in_block = False @@ -89,7 +86,6 @@ class BadTxTemplate: class OutputMissing(BadTxTemplate): reject_reason = "bad-txns-vout-empty" - expect_disconnect = True def get_tx(self): tx = CTransaction() @@ -100,7 +96,6 @@ class OutputMissing(BadTxTemplate): class InputMissing(BadTxTemplate): reject_reason = "bad-txns-vin-empty" - expect_disconnect = True # We use a blank transaction here to make sure # it is interpreted as a non-witness transaction. @@ -134,7 +129,6 @@ class BadInputOutpointIndex(BadTxTemplate): # Won't be rejected - nonexistent outpoint index is treated as an orphan since the coins # database can't distinguish between spent outpoints and outpoints which never existed. reject_reason = None - expect_disconnect = False def get_tx(self): num_indices = len(self.spend_tx.vin) @@ -149,7 +143,6 @@ class BadInputOutpointIndex(BadTxTemplate): class DuplicateInput(BadTxTemplate): reject_reason = 'bad-txns-inputs-duplicate' - expect_disconnect = True def get_tx(self): tx = CTransaction() @@ -162,7 +155,6 @@ class DuplicateInput(BadTxTemplate): class PrevoutNullInput(BadTxTemplate): reject_reason = 'bad-txns-prevout-null' - expect_disconnect = True def get_tx(self): tx = CTransaction() @@ -175,7 +167,6 @@ class PrevoutNullInput(BadTxTemplate): class NonexistentInput(BadTxTemplate): reject_reason = None # Added as an orphan tx. - expect_disconnect = False def get_tx(self): tx = CTransaction() @@ -189,7 +180,6 @@ class NonexistentInput(BadTxTemplate): class SpendTooMuch(BadTxTemplate): reject_reason = 'bad-txns-in-ne-out' block_reject_reason = 'block-validation-failed' - expect_disconnect = True def get_tx(self): return create_tx_with_script( @@ -198,7 +188,6 @@ class SpendTooMuch(BadTxTemplate): class CreateNegative(BadTxTemplate): reject_reason = 'bad-txns-vout-negative' - expect_disconnect = True def get_tx(self): return create_tx_with_script(self.spend_tx, 0, amount=-1) @@ -206,7 +195,6 @@ class CreateNegative(BadTxTemplate): class CreateTooLarge(BadTxTemplate): reject_reason = 'bad-txns-vout-toolarge' - expect_disconnect = True def get_tx(self): return create_tx_with_script(self.spend_tx, 0, amount=MAX_MONEY + 1) @@ -214,7 +202,6 @@ class CreateTooLarge(BadTxTemplate): class CreateSumTooLarge(BadTxTemplate): reject_reason = 'bad-txns-txouttotal-toolarge' - expect_disconnect = True def get_tx(self): tx = create_tx_with_script(self.spend_tx, 0, amount=MAX_MONEY) @@ -224,8 +211,7 @@ class CreateSumTooLarge(BadTxTemplate): class InvalidOPIFConstruction(BadTxTemplate): - reject_reason = "mandatory-script-verify-flag-failed (Invalid OP_IF construction)" - expect_disconnect = True + reject_reason = "mempool-script-verify-flag-failed (Invalid OP_IF construction)" valid_in_block = True def get_tx(self): @@ -237,7 +223,6 @@ class InvalidOPIFConstruction(BadTxTemplate): class TooManySigops(BadTxTemplate): reject_reason = "bad-txns-too-many-sigops" block_reject_reason = "bad-blk-sigops, out-of-bounds SigOpCount" - expect_disconnect = False def get_tx(self): lotsa_checksigs = CScript([OP_CHECKSIG] * (MAX_BLOCK_SIGOPS)) @@ -260,7 +245,6 @@ def getDisabledOpcodeTemplate(opcode): return type('DisabledOpcode_' + str(opcode), (BadTxTemplate,), { 'reject_reason': "disabled opcode", - 'expect_disconnect': True, 'get_tx': get_tx, 'valid_in_block' : True }) diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index f906f29250..3030d94ec3 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -168,9 +168,12 @@ class FullBlockTest(BitcoinTestFramework): self.sign_tx(badtx, attempt_spend_tx) badtx.rehash() badblock = self.update_block(blockname, [badtx]) + reject_reason = (template.block_reject_reason or template.reject_reason) + if reject_reason == "mempool-script-verify-flag-failed": + reject_reason = "mandatory-script-verify-flag-failed" + reject_reason[33:] self.send_blocks( [badblock], success=False, - reject_reason=(template.block_reject_reason or template.reject_reason), + reject_reason=reject_reason, reconnect=True, timeout=2) self.move_tip(2) diff --git a/test/functional/feature_cltv.py b/test/functional/feature_cltv.py index 4bcbe27189..0f47a1656c 100755 --- a/test/functional/feature_cltv.py +++ b/test/functional/feature_cltv.py @@ -150,12 +150,13 @@ class BIP65Test(BitcoinTestFramework): spendtx = wallet.create_self_transfer()['tx'] cltv_invalidate(spendtx, i) + tx_rej = "mempool-script-verify-flag-failed" expected_cltv_reject_reason = [ - "non-mandatory-script-verify-flag (Operation not valid with the current stack size)", - "non-mandatory-script-verify-flag (Negative locktime)", - "non-mandatory-script-verify-flag (Locktime requirement not satisfied)", - "non-mandatory-script-verify-flag (Locktime requirement not satisfied)", - "non-mandatory-script-verify-flag (Locktime requirement not satisfied)", + " (Operation not valid with the current stack size)", + " (Negative locktime)", + " (Locktime requirement not satisfied)", + " (Locktime requirement not satisfied)", + " (Locktime requirement not satisfied)", ][i] # First we show that this tx is valid except for CLTV by getting it # rejected from the mempool for exactly that reason. @@ -164,7 +165,7 @@ class BIP65Test(BitcoinTestFramework): 'txid': spendtx.hash, 'wtxid': spendtx.getwtxid(), 'allowed': False, - 'reject-reason': expected_cltv_reject_reason, + 'reject-reason': tx_rej + expected_cltv_reject_reason, }], self.nodes[0].testmempoolaccept(rawtxs=[spendtx.serialize().hex()], maxfeerate=0), ) @@ -174,7 +175,7 @@ class BIP65Test(BitcoinTestFramework): block.hashMerkleRoot = block.calc_merkle_root() block.solve() - with self.nodes[0].assert_debug_log(expected_msgs=[f'CheckInputScripts on {block.vtx[-1].hash} failed with {expected_cltv_reject_reason}']): + with self.nodes[0].assert_debug_log(expected_msgs=[f'CheckInputScripts on {block.vtx[-1].hash} failed with {tx_rej + expected_cltv_reject_reason}']): peer.send_and_ping(msg_block(block)) assert_equal(int(self.nodes[0].getbestblockhash(), 16), tip) peer.sync_with_ping() diff --git a/test/functional/feature_dersig.py b/test/functional/feature_dersig.py index d39002be05..52acd2af49 100755 --- a/test/functional/feature_dersig.py +++ b/test/functional/feature_dersig.py @@ -121,7 +121,7 @@ class BIP66Test(BitcoinTestFramework): 'txid': spendtx.hash, 'wtxid': spendtx.getwtxid(), 'allowed': False, - 'reject-reason': 'non-mandatory-script-verify-flag (Non-canonical DER signature)', + 'reject-reason': 'mempool-script-verify-flag-failed (Non-canonical DER signature)', }], self.nodes[0].testmempoolaccept(rawtxs=[spendtx.serialize().hex()], maxfeerate=0), ) @@ -131,7 +131,7 @@ class BIP66Test(BitcoinTestFramework): block.hashMerkleRoot = block.calc_merkle_root() block.solve() - with self.nodes[0].assert_debug_log(expected_msgs=[f'CheckInputScripts on {block.vtx[-1].hash} failed with non-mandatory-script-verify-flag (Non-canonical DER signature)']): + with self.nodes[0].assert_debug_log(expected_msgs=[f'CheckInputScripts on {block.vtx[-1].hash} failed with mempool-script-verify-flag-failed (Non-canonical DER signature)']): peer.send_and_ping(msg_block(block)) assert_equal(int(self.nodes[0].getbestblockhash(), 16), tip) peer.sync_with_ping() diff --git a/test/functional/feature_nulldummy.py b/test/functional/feature_nulldummy.py index 00ff8210eb..9c0a56f23c 100755 --- a/test/functional/feature_nulldummy.py +++ b/test/functional/feature_nulldummy.py @@ -33,8 +33,7 @@ from test_framework.util import ( assert_raises_rpc_error, ) -NULLDUMMY_ERROR = "non-mandatory-script-verify-flag (Dummy CHECKMULTISIG argument must be zero)" - +NULLDUMMY_ERROR = "mempool-script-verify-flag-failed (Dummy CHECKMULTISIG argument must be zero)" def invalidate_nulldummy_tx(tx): """Transform a NULLDUMMY compliant tx (i.e. scriptSig starts with OP_0) diff --git a/test/functional/feature_segwit.py b/test/functional/feature_segwit.py index 5cf55b029e..d91e6ffbc0 100755 --- a/test/functional/feature_segwit.py +++ b/test/functional/feature_segwit.py @@ -207,8 +207,8 @@ class SegWitTest(BitcoinTestFramework): self.skip_mine(self.nodes[2], p2sh_ids[NODE_2][P2WSH][0], True) # block 427 self.log.info("Verify unsigned p2sh witness txs without a redeem script are invalid") - self.fail_accept(self.nodes[2], "mandatory-script-verify-flag-failed (Operation not valid with the current stack size)", p2sh_ids[NODE_2][P2WPKH][1], sign=False) - self.fail_accept(self.nodes[2], "mandatory-script-verify-flag-failed (Operation not valid with the current stack size)", p2sh_ids[NODE_2][P2WSH][1], sign=False) + self.fail_accept(self.nodes[2], "mempool-script-verify-flag-failed (Operation not valid with the current stack size)", p2sh_ids[NODE_2][P2WPKH][1], sign=False) + self.fail_accept(self.nodes[2], "mempool-script-verify-flag-failed (Operation not valid with the current stack size)", p2sh_ids[NODE_2][P2WSH][1], sign=False) self.generate(self.nodes[2], 4) # blocks 428-431 @@ -221,13 +221,13 @@ class SegWitTest(BitcoinTestFramework): self.log.info("Verify default node can't accept txs with missing witness") # unsigned, no scriptsig - self.fail_accept(self.nodes[0], "non-mandatory-script-verify-flag (Witness program hash mismatch)", wit_ids[NODE_0][P2WPKH][0], sign=False) - self.fail_accept(self.nodes[0], "non-mandatory-script-verify-flag (Witness program was passed an empty witness)", wit_ids[NODE_0][P2WSH][0], sign=False) - self.fail_accept(self.nodes[0], "mandatory-script-verify-flag-failed (Operation not valid with the current stack size)", p2sh_ids[NODE_0][P2WPKH][0], sign=False) - self.fail_accept(self.nodes[0], "mandatory-script-verify-flag-failed (Operation not valid with the current stack size)", p2sh_ids[NODE_0][P2WSH][0], sign=False) + self.fail_accept(self.nodes[0], "mempool-script-verify-flag-failed (Witness program hash mismatch)", wit_ids[NODE_0][P2WPKH][0], sign=False) + self.fail_accept(self.nodes[0], "mempool-script-verify-flag-failed (Witness program was passed an empty witness)", wit_ids[NODE_0][P2WSH][0], sign=False) + self.fail_accept(self.nodes[0], "mempool-script-verify-flag-failed (Operation not valid with the current stack size)", p2sh_ids[NODE_0][P2WPKH][0], sign=False) + self.fail_accept(self.nodes[0], "mempool-script-verify-flag-failed (Operation not valid with the current stack size)", p2sh_ids[NODE_0][P2WSH][0], sign=False) # unsigned with redeem script - self.fail_accept(self.nodes[0], "non-mandatory-script-verify-flag (Witness program hash mismatch)", p2sh_ids[NODE_0][P2WPKH][0], sign=False, redeem_script=witness_script(False, self.pubkey[0])) - self.fail_accept(self.nodes[0], "non-mandatory-script-verify-flag (Witness program was passed an empty witness)", p2sh_ids[NODE_0][P2WSH][0], sign=False, redeem_script=witness_script(True, self.pubkey[0])) + self.fail_accept(self.nodes[0], "mempool-script-verify-flag-failed (Witness program hash mismatch)", p2sh_ids[NODE_0][P2WPKH][0], sign=False, redeem_script=witness_script(False, self.pubkey[0])) + self.fail_accept(self.nodes[0], "mempool-script-verify-flag-failed (Witness program was passed an empty witness)", p2sh_ids[NODE_0][P2WSH][0], sign=False, redeem_script=witness_script(True, self.pubkey[0])) self.log.info("Verify block and transaction serialization rpcs return differing serializations depending on rpc serialization flag") assert self.nodes[2].getblock(blockhash, False) != self.nodes[0].getblock(blockhash, False) @@ -250,10 +250,10 @@ class SegWitTest(BitcoinTestFramework): assert_equal(witnesses[0], '00' * 32) self.log.info("Verify witness txs without witness data are invalid after the fork") - self.fail_accept(self.nodes[2], 'non-mandatory-script-verify-flag (Witness program hash mismatch)', wit_ids[NODE_2][P2WPKH][2], sign=False) - self.fail_accept(self.nodes[2], 'non-mandatory-script-verify-flag (Witness program was passed an empty witness)', wit_ids[NODE_2][P2WSH][2], sign=False) - self.fail_accept(self.nodes[2], 'non-mandatory-script-verify-flag (Witness program hash mismatch)', p2sh_ids[NODE_2][P2WPKH][2], sign=False, redeem_script=witness_script(False, self.pubkey[2])) - self.fail_accept(self.nodes[2], 'non-mandatory-script-verify-flag (Witness program was passed an empty witness)', p2sh_ids[NODE_2][P2WSH][2], sign=False, redeem_script=witness_script(True, self.pubkey[2])) + self.fail_accept(self.nodes[2], 'mempool-script-verify-flag-failed (Witness program hash mismatch)', wit_ids[NODE_2][P2WPKH][2], sign=False) + self.fail_accept(self.nodes[2], 'mempool-script-verify-flag-failed (Witness program was passed an empty witness)', wit_ids[NODE_2][P2WSH][2], sign=False) + self.fail_accept(self.nodes[2], 'mempool-script-verify-flag-failed (Witness program hash mismatch)', p2sh_ids[NODE_2][P2WPKH][2], sign=False, redeem_script=witness_script(False, self.pubkey[2])) + self.fail_accept(self.nodes[2], 'mempool-script-verify-flag-failed (Witness program was passed an empty witness)', p2sh_ids[NODE_2][P2WSH][2], sign=False, redeem_script=witness_script(True, self.pubkey[2])) self.log.info("Verify default node can now use witness txs") self.success_mine(self.nodes[0], wit_ids[NODE_0][P2WPKH][0], True) # block 432 diff --git a/test/functional/p2p_invalid_tx.py b/test/functional/p2p_invalid_tx.py index 88e7d0ba59..1ad809c2b5 100755 --- a/test/functional/p2p_invalid_tx.py +++ b/test/functional/p2p_invalid_tx.py @@ -74,14 +74,9 @@ class InvalidTxRequestTest(BitcoinTestFramework): tx = template.get_tx() node.p2ps[0].send_txs_and_test( [tx], node, success=False, - expect_disconnect=template.expect_disconnect, reject_reason=template.reject_reason, ) - if template.expect_disconnect: - self.log.info("Reconnecting to peer") - self.reconnect_p2p() - # Make two p2p connections to provide the node with orphans # * p2ps[0] will send valid orphan txs (one with low fee) # * p2ps[1] will send an invalid orphan tx (and is later disconnected for that) @@ -148,7 +143,6 @@ class InvalidTxRequestTest(BitcoinTestFramework): # tx_orphan_2_no_fee, because it has too low fee (p2ps[0] is not disconnected for relaying that tx) # tx_orphan_2_invalid, because it has negative fee (p2ps[1] is disconnected for relaying that tx) - self.wait_until(lambda: 1 == len(node.getpeerinfo()), timeout=12) # p2ps[1] is no longer connected assert_equal(expected_mempool, set(node.getrawmempool())) self.log.info('Test orphan pool overflow') diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index 60c447ffcc..e1e75c5334 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -518,10 +518,10 @@ class SegWitTest(BitcoinTestFramework): # without a witness is invalid). # Note: The reject reason for this failure could be # 'block-validation-failed' (if script check threads > 1) or - # 'non-mandatory-script-verify-flag (Witness program was passed an + # 'mempool-script-verify-flag-failed (Witness program was passed an # empty witness)' (otherwise). test_witness_block(self.nodes[0], self.test_node, block, accepted=False, with_witness=False, - reason='non-mandatory-script-verify-flag (Witness program was passed an empty witness)') + reason='mempool-script-verify-flag-failed (Witness program was passed an empty witness)') self.utxo.pop(0) self.utxo.append(UTXO(txid, 2, value)) @@ -718,7 +718,7 @@ class SegWitTest(BitcoinTestFramework): # segwit activation. Note that older bitcoind's that are not # segwit-aware would also reject this for failing CLEANSTACK. with self.nodes[0].assert_debug_log( - expected_msgs=(spend_tx.hash, 'was not accepted: non-mandatory-script-verify-flag (Witness program was passed an empty witness)')): + expected_msgs=(spend_tx.hash, 'was not accepted: mempool-script-verify-flag-failed (Witness program was passed an empty witness)')): test_transaction_acceptance(self.nodes[0], self.test_node, spend_tx, with_witness=False, accepted=False) # The transaction was detected as witness stripped above and not added to the reject @@ -731,7 +731,7 @@ class SegWitTest(BitcoinTestFramework): spend_tx.vin[0].scriptSig = CScript([p2wsh_pubkey, b'a']) spend_tx.rehash() with self.nodes[0].assert_debug_log( - expected_msgs=(spend_tx.hash, 'was not accepted: mandatory-script-verify-flag-failed (Script evaluated without error but finished with a false/empty top stack element)')): + expected_msgs=(spend_tx.hash, 'was not accepted: mempool-script-verify-flag-failed (Script evaluated without error but finished with a false/empty top stack element)')): test_transaction_acceptance(self.nodes[0], self.test_node, spend_tx, with_witness=False, accepted=False) # Now put the witness script in the witness, should succeed after @@ -1025,7 +1025,7 @@ class SegWitTest(BitcoinTestFramework): # Extra witness data should not be allowed. test_witness_block(self.nodes[0], self.test_node, block, accepted=False, - reason='non-mandatory-script-verify-flag (Witness provided for non-witness script)') + reason='mempool-script-verify-flag-failed (Witness provided for non-witness script)') # Try extra signature data. Ok if we're not spending a witness output. block.vtx[1].wit.vtxinwit = [] @@ -1052,7 +1052,7 @@ class SegWitTest(BitcoinTestFramework): # This has extra witness data, so it should fail. test_witness_block(self.nodes[0], self.test_node, block, accepted=False, - reason='non-mandatory-script-verify-flag (Stack size must be exactly one after execution)') + reason='mempool-script-verify-flag-failed (Stack size must be exactly one after execution)') # Now get rid of the extra witness, but add extra scriptSig data tx2.vin[0].scriptSig = CScript([OP_TRUE]) @@ -1065,7 +1065,7 @@ class SegWitTest(BitcoinTestFramework): # This has extra signature data for a witness input, so it should fail. test_witness_block(self.nodes[0], self.test_node, block, accepted=False, - reason='non-mandatory-script-verify-flag (Witness requires empty scriptSig)') + reason='mempool-script-verify-flag-failed (Witness requires empty scriptSig)') # Now get rid of the extra scriptsig on the witness input, and verify # success (even with extra scriptsig data in the non-witness input) @@ -1106,7 +1106,7 @@ class SegWitTest(BitcoinTestFramework): self.update_witness_block_with_transactions(block, [tx, tx2]) test_witness_block(self.nodes[0], self.test_node, block, accepted=False, - reason='non-mandatory-script-verify-flag (Push value size limit exceeded)') + reason='mempool-script-verify-flag-failed (Push value size limit exceeded)') # Now reduce the length of the stack element tx2.wit.vtxinwit[0].scriptWitness.stack[0] = b'a' * (MAX_SCRIPT_ELEMENT_SIZE) @@ -1149,7 +1149,7 @@ class SegWitTest(BitcoinTestFramework): self.update_witness_block_with_transactions(block, [tx, tx2]) test_witness_block(self.nodes[0], self.test_node, block, accepted=False, - reason='non-mandatory-script-verify-flag (Script is too big)') + reason='mempool-script-verify-flag-failed (Script is too big)') # Try again with one less byte in the witness script witness_script = CScript([b'a' * MAX_SCRIPT_ELEMENT_SIZE] * 19 + [OP_DROP] * 62 + [OP_TRUE]) @@ -1244,7 +1244,7 @@ class SegWitTest(BitcoinTestFramework): block.vtx = [block.vtx[0]] self.update_witness_block_with_transactions(block, [tx2]) test_witness_block(self.nodes[0], self.test_node, block, accepted=False, - reason='non-mandatory-script-verify-flag (Operation not valid with the current stack size)') + reason='mempool-script-verify-flag-failed (Operation not valid with the current stack size)') # Fix the broken witness and the block should be accepted. tx2.wit.vtxinwit[5].scriptWitness.stack = [b'a', witness_script] @@ -1535,7 +1535,7 @@ class SegWitTest(BitcoinTestFramework): tx2.rehash() # Should fail policy test. - test_transaction_acceptance(self.nodes[0], self.test_node, tx2, True, False, 'non-mandatory-script-verify-flag (Using non-compressed keys in segwit)') + test_transaction_acceptance(self.nodes[0], self.test_node, tx2, True, False, 'mempool-script-verify-flag-failed (Using non-compressed keys in segwit)') # But passes consensus. block = self.build_next_block() self.update_witness_block_with_transactions(block, [tx2]) @@ -1555,7 +1555,7 @@ class SegWitTest(BitcoinTestFramework): sign_p2pk_witness_input(witness_script, tx3, 0, SIGHASH_ALL, tx2.vout[0].nValue.getAmount(), key) # Should fail policy test. - test_transaction_acceptance(self.nodes[0], self.test_node, tx3, True, False, 'non-mandatory-script-verify-flag (Using non-compressed keys in segwit)') + test_transaction_acceptance(self.nodes[0], self.test_node, tx3, True, False, 'mempool-script-verify-flag-failed (Using non-compressed keys in segwit)') # But passes consensus. block = self.build_next_block() self.update_witness_block_with_transactions(block, [tx3]) @@ -1573,7 +1573,7 @@ class SegWitTest(BitcoinTestFramework): sign_p2pk_witness_input(witness_script, tx4, 0, SIGHASH_ALL, tx3.vout[0].nValue.getAmount(), key) # Should fail policy test. - test_transaction_acceptance(self.nodes[0], self.test_node, tx4, True, False, 'non-mandatory-script-verify-flag (Using non-compressed keys in segwit)') + test_transaction_acceptance(self.nodes[0], self.test_node, tx4, True, False, 'mempool-script-verify-flag-failed (Using non-compressed keys in segwit)') block = self.build_next_block() self.update_witness_block_with_transactions(block, [tx4]) test_witness_block(self.nodes[0], self.test_node, block, accepted=True) @@ -1635,7 +1635,7 @@ class SegWitTest(BitcoinTestFramework): sign_p2pk_witness_input(witness_script, tx, 0, hashtype, prev_utxo.nValue + 1, key) self.update_witness_block_with_transactions(block, [tx]) test_witness_block(self.nodes[0], self.test_node, block, accepted=False, - reason='non-mandatory-script-verify-flag (Script evaluated without error ' + reason='mempool-script-verify-flag-failed (Script evaluated without error ' 'but finished with a false/empty top stack element') # Too-small input value @@ -1643,7 +1643,7 @@ class SegWitTest(BitcoinTestFramework): block.vtx.pop() # remove last tx self.update_witness_block_with_transactions(block, [tx]) test_witness_block(self.nodes[0], self.test_node, block, accepted=False, - reason='non-mandatory-script-verify-flag (Script evaluated without error ' + reason='mempool-script-verify-flag-failed (Script evaluated without error ' 'but finished with a false/empty top stack element') # Now try correct value @@ -1750,7 +1750,7 @@ class SegWitTest(BitcoinTestFramework): block = self.build_next_block() self.update_witness_block_with_transactions(block, [tx, tx2]) test_witness_block(self.nodes[0], self.test_node, block, accepted=False, - reason='non-mandatory-script-verify-flag (Witness requires empty scriptSig)') + reason='mempool-script-verify-flag-failed (Witness requires empty scriptSig)') # Move the signature to the witness. block.vtx.pop() diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py index bf93639573..f45d80cc3c 100755 --- a/test/functional/rpc_packages.py +++ b/test/functional/rpc_packages.py @@ -112,7 +112,7 @@ class RPCPackagesTest(BitcoinTestFramework): assert_equal(testres_bad_sig, self.independent_txns_testres + [{ "txid": tx_bad_sig.rehash(), "wtxid": tx_bad_sig.getwtxid(), "allowed": False, - "reject-reason": "mandatory-script-verify-flag-failed (Operation not valid with the current stack size)" + "reject-reason": "mempool-script-verify-flag-failed (Operation not valid with the current stack size)" }]) self.log.info("Check testmempoolaccept reports txns in packages that exceed max feerate") diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index 840670ae36..95be64908a 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -735,13 +735,12 @@ class P2PDataStore(P2PInterface): else: assert node.getbestblockhash() != blocks[-1].hash - def send_txs_and_test(self, txs, node, *, success=True, expect_disconnect=False, reject_reason=None): + def send_txs_and_test(self, txs, node, *, success=True, reject_reason=None): """Send txs to test node and test whether they're accepted to the mempool. - add all txs to our tx_store - send tx messages for all txs - if success is True/False: assert that the txs are/are not accepted to the mempool - - if expect_disconnect is True: Skip the sync with ping - if reject_reason is set: assert that the correct reject message is logged.""" with p2p_lock: @@ -753,10 +752,7 @@ class P2PDataStore(P2PInterface): for tx in txs: self.send_message(msg_tx(tx)) - if expect_disconnect: - self.wait_for_disconnect() - else: - self.sync_with_ping() + self.sync_with_ping() raw_mempool = node.getrawmempool() if success: