diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 61439f71883..ec6f55cfdf6 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -583,12 +583,6 @@ private: bool via_compact_block, const std::string& message = "") EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); - /** - * Potentially disconnect and discourage a node based on the contents of a TxValidationState object - */ - void MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state) - EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex); - /** Maybe disconnect a peer and discourage future connections from its address. * * @param[in] pnode The node to check. @@ -1836,32 +1830,6 @@ void PeerManagerImpl::MaybePunishNodeForBlock(NodeId nodeid, const BlockValidati } } -void PeerManagerImpl::MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state) -{ - PeerRef peer{GetPeerRef(nodeid)}; - switch (state.GetResult()) { - case TxValidationResult::TX_RESULT_UNSET: - break; - // The node is providing invalid data: - case TxValidationResult::TX_CONSENSUS: - if (peer) Misbehaving(*peer, ""); - return; - // Conflicting (but not necessarily invalid) data or different policy: - 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: - case TxValidationResult::TX_RECONSIDERABLE: - case TxValidationResult::TX_UNKNOWN: - break; - } -} - bool PeerManagerImpl::BlockRequestAllowed(const CBlockIndex* pindex) { AssertLockHeld(cs_main); @@ -3034,8 +3002,6 @@ std::optional PeerManagerImpl::ProcessInvalidTx(NodeId if (peer) AddKnownTx(*peer, parent_txid); } - MaybePunishNodeForTx(nodeid, state); - return package_to_validate; } diff --git a/test/functional/data/invalid_txs.py b/test/functional/data/invalid_txs.py index 36b274efc27..32a188d19bf 100644 --- a/test/functional/data/invalid_txs.py +++ b/test/functional/data/invalid_txs.py @@ -93,7 +93,7 @@ class BadTxTemplate: class OutputMissing(BadTxTemplate): reject_reason = "bad-txns-vout-empty" - expect_disconnect = True + expect_disconnect = False def get_tx(self): tx = CTransaction() @@ -103,7 +103,7 @@ class OutputMissing(BadTxTemplate): class InputMissing(BadTxTemplate): reject_reason = "bad-txns-vin-empty" - expect_disconnect = True + expect_disconnect = False # We use a blank transaction here to make sure # it is interpreted as a non-witness transaction. @@ -151,7 +151,7 @@ class BadInputOutpointIndex(BadTxTemplate): class DuplicateInput(BadTxTemplate): reject_reason = 'bad-txns-inputs-duplicate' - expect_disconnect = True + expect_disconnect = False def get_tx(self): tx = CTransaction() @@ -163,7 +163,7 @@ class DuplicateInput(BadTxTemplate): class PrevoutNullInput(BadTxTemplate): reject_reason = 'bad-txns-prevout-null' - expect_disconnect = True + expect_disconnect = False def get_tx(self): tx = CTransaction() @@ -189,7 +189,7 @@ class NonexistentInput(BadTxTemplate): class SpendTooMuch(BadTxTemplate): reject_reason = 'bad-txns-in-belowout' - expect_disconnect = True + expect_disconnect = False def get_tx(self): return create_tx_with_script( @@ -198,7 +198,7 @@ class SpendTooMuch(BadTxTemplate): class CreateNegative(BadTxTemplate): reject_reason = 'bad-txns-vout-negative' - expect_disconnect = True + expect_disconnect = False def get_tx(self): return create_tx_with_script(self.spend_tx, 0, amount=-1) @@ -206,7 +206,7 @@ class CreateNegative(BadTxTemplate): class CreateTooLarge(BadTxTemplate): reject_reason = 'bad-txns-vout-toolarge' - expect_disconnect = True + expect_disconnect = False def get_tx(self): return create_tx_with_script(self.spend_tx, 0, amount=MAX_MONEY + 1) @@ -214,7 +214,7 @@ class CreateTooLarge(BadTxTemplate): class CreateSumTooLarge(BadTxTemplate): reject_reason = 'bad-txns-txouttotal-toolarge' - expect_disconnect = True + expect_disconnect = False def get_tx(self): tx = create_tx_with_script(self.spend_tx, 0, amount=MAX_MONEY) @@ -224,7 +224,7 @@ class CreateSumTooLarge(BadTxTemplate): class InvalidOPIFConstruction(BadTxTemplate): reject_reason = "mandatory-script-verify-flag-failed (Invalid OP_IF construction)" - expect_disconnect = True + expect_disconnect = False def get_tx(self): return create_tx_with_script( @@ -278,7 +278,7 @@ def getDisabledOpcodeTemplate(opcode): class NonStandardAndInvalid(BadTxTemplate): """A non-standard transaction which is also consensus-invalid should return the consensus error.""" reject_reason = "mandatory-script-verify-flag-failed (OP_RETURN was encountered)" - expect_disconnect = True + expect_disconnect = False valid_in_block = False def get_tx(self): diff --git a/test/functional/p2p_invalid_tx.py b/test/functional/p2p_invalid_tx.py index 634dbc9e26c..b041c55bfd4 100755 --- a/test/functional/p2p_invalid_tx.py +++ b/test/functional/p2p_invalid_tx.py @@ -73,7 +73,7 @@ class InvalidTxRequestTest(BitcoinTestFramework): tx = template.get_tx() node.p2ps[0].send_txs_and_test( [tx], node, success=False, - expect_disconnect=template.expect_disconnect, + expect_disconnect=False, reject_reason=template.reject_reason, ) @@ -140,7 +140,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 orphanage can store more than 100 transactions') diff --git a/test/functional/p2p_opportunistic_1p1c.py b/test/functional/p2p_opportunistic_1p1c.py index ad42b7308ba..d75d6b40e20 100755 --- a/test/functional/p2p_opportunistic_1p1c.py +++ b/test/functional/p2p_opportunistic_1p1c.py @@ -274,8 +274,10 @@ class PackageRelayTest(BitcoinTestFramework): assert tx_orphan_bad_wit.txid_hex not in node_mempool # 5. Have the other peer send the tx too, so that tx_orphan_bad_wit package is attempted. - bad_orphan_sender.send_without_ping(msg_tx(low_fee_parent["tx"])) - bad_orphan_sender.wait_for_disconnect() + bad_orphan_sender.send_and_ping(msg_tx(low_fee_parent["tx"])) + + # The bad orphan sender should not be disconnected. + bad_orphan_sender.sync_with_ping() # The peer that didn't provide the orphan should not be disconnected. parent_sender.sync_with_ping()