tests: drop expect_disconnect behaviour for tx relay
What changed, and why it matters
This commit only changes Bitcoin Core's internal functional test code. It removes an old test helper flag called expect_disconnect that was used when sending invalid transactions during tests. The production Bitcoin node code that actually validates transactions and decides whether to disconnect peers is not changed at all. Therefore this commit does not fix or introduce any security issue in the live Bitcoin software.
No security action required. Treat as a normal test-maintenance commit. If reviewing, confirm that the removed expect_disconnect behavior is no longer needed because the node no longer disconnects for these transactions, or that the test coverage is preserved elsewhere.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The diff removes the expect_disconnect attribute from invalid transaction templates in test/functional/data/invalid_txs.py and drops the corresponding parameter from send_txs_and_test() in the test framework. Previously, some disabled-opcode templates set expect_disconnect=True, causing tests to wait for a peer disconnect instead of syncing with a ping. The commit unifies the test path so all invalid-transaction tests sync with ping and no test expects a disconnect. This is purely a test-code refactor; net_processing.cpp and related validation/disconnection logic are untouched.
Changed components
test/functional/data/invalid_txs.pytest/functional/feature_block.pytest/functional/p2p_invalid_tx.pytest/functional/test_framework/p2p.pyInspect captured patch +2 / −35
diff --git a/test/functional/data/invalid_txs.py b/test/functional/data/invalid_txs.py
index 3c249b2f..a283945a 100644
--- a/test/functional/data/invalid_txs.py
+++ b/test/functional/data/invalid_txs.py
@@ -73,9 +73,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
@@ -93,7 +90,6 @@ class BadTxTemplate:
class OutputMissing(BadTxTemplate):
reject_reason = "bad-txns-vout-empty"
- expect_disconnect = False
def get_tx(self):
tx = CTransaction()
@@ -103,7 +99,6 @@ class OutputMissing(BadTxTemplate):
class InputMissing(BadTxTemplate):
reject_reason = "bad-txns-vin-empty"
- expect_disconnect = False
# We use a blank transaction here to make sure
# it is interpreted as a non-witness transaction.
@@ -119,7 +114,6 @@ class InputMissing(BadTxTemplate):
# tree depth commitment (CVE-2017-12842)
class SizeTooSmall(BadTxTemplate):
reject_reason = "tx-size-small"
- expect_disconnect = False
valid_in_block = True
def get_tx(self):
@@ -137,7 +131,6 @@ class BadInputOutpointIndex(BadTxTemplate):
reject_reason = None
# But fails in block
block_reject_reason = "bad-txns-inputs-missingorspent"
- expect_disconnect = False
def get_tx(self):
num_indices = len(self.spend_tx.vin)
@@ -151,7 +144,6 @@ class BadInputOutpointIndex(BadTxTemplate):
class DuplicateInput(BadTxTemplate):
reject_reason = 'bad-txns-inputs-duplicate'
- expect_disconnect = False
def get_tx(self):
tx = CTransaction()
@@ -163,7 +155,6 @@ class DuplicateInput(BadTxTemplate):
class PrevoutNullInput(BadTxTemplate):
reject_reason = 'bad-txns-prevout-null'
- expect_disconnect = False
def get_tx(self):
tx = CTransaction()
@@ -175,7 +166,6 @@ class PrevoutNullInput(BadTxTemplate):
class NonexistentInput(BadTxTemplate):
reject_reason = None # Added as an orphan tx.
- expect_disconnect = False
# But fails in block
block_reject_reason = "bad-txns-inputs-missingorspent"
@@ -189,7 +179,6 @@ class NonexistentInput(BadTxTemplate):
class SpendTooMuch(BadTxTemplate):
reject_reason = 'bad-txns-in-belowout'
- expect_disconnect = False
def get_tx(self):
return create_tx_with_script(
@@ -198,7 +187,6 @@ class SpendTooMuch(BadTxTemplate):
class CreateNegative(BadTxTemplate):
reject_reason = 'bad-txns-vout-negative'
- expect_disconnect = False
def get_tx(self):
return create_tx_with_script(self.spend_tx, 0, amount=-1)
@@ -206,7 +194,6 @@ class CreateNegative(BadTxTemplate):
class CreateTooLarge(BadTxTemplate):
reject_reason = 'bad-txns-vout-toolarge'
- expect_disconnect = False
def get_tx(self):
return create_tx_with_script(self.spend_tx, 0, amount=MAX_MONEY + 1)
@@ -214,7 +201,6 @@ class CreateTooLarge(BadTxTemplate):
class CreateSumTooLarge(BadTxTemplate):
reject_reason = 'bad-txns-txouttotal-toolarge'
- expect_disconnect = False
def get_tx(self):
tx = create_tx_with_script(self.spend_tx, 0, amount=MAX_MONEY)
@@ -224,7 +210,6 @@ class CreateSumTooLarge(BadTxTemplate):
class InvalidOPIFConstruction(BadTxTemplate):
reject_reason = "mempool-script-verify-flag-failed (Invalid OP_IF construction)"
- expect_disconnect = False
def get_tx(self):
return create_tx_with_script(
@@ -235,7 +220,6 @@ class InvalidOPIFConstruction(BadTxTemplate):
class TooManySigopsPerBlock(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))
@@ -247,7 +231,6 @@ class TooManySigopsPerBlock(BadTxTemplate):
class TooManySigopsPerTransaction(BadTxTemplate):
reject_reason = "bad-txns-too-many-sigops"
- expect_disconnect = False
valid_in_block = True
def get_tx(self):
@@ -270,7 +253,6 @@ def getDisabledOpcodeTemplate(opcode):
return type('DisabledOpcode_' + str(opcode), (BadTxTemplate,), {
'reject_reason': "disabled opcode",
- 'expect_disconnect': True,
'get_tx': get_tx,
'valid_in_block' : False
})
@@ -279,7 +261,6 @@ class NonStandardAndInvalid(BadTxTemplate):
"""A non-standard transaction which is also consensus-invalid should return the first error."""
reject_reason = "mempool-script-verify-flag-failed (Using OP_CODESEPARATOR in non-witness script)"
block_reject_reason = "mandatory-script-verify-flag-failed (OP_RETURN was encountered)"
- expect_disconnect = False
valid_in_block = False
def get_tx(self):
diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py
index bc0364a8..63bee495 100755
--- a/test/functional/feature_block.py
+++ b/test/functional/feature_block.py
@@ -177,11 +177,6 @@ class FullBlockTest(BitcoinTestFramework):
for TxTemplate in invalid_txs.iter_all_templates():
template = TxTemplate(spend_tx=attempt_spend_tx)
- # belt-and-suspenders checking we won't pass up validating something
- # we expect a disconnect from
- if template.expect_disconnect:
- assert not template.valid_in_block
-
if template.valid_in_block:
continue
diff --git a/test/functional/p2p_invalid_tx.py b/test/functional/p2p_invalid_tx.py
index b041c55b..0d4c4ffb 100755
--- a/test/functional/p2p_invalid_tx.py
+++ b/test/functional/p2p_invalid_tx.py
@@ -73,14 +73,9 @@ class InvalidTxRequestTest(BitcoinTestFramework):
tx = template.get_tx()
node.p2ps[0].send_txs_and_test(
[tx], node, success=False,
- expect_disconnect=False,
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)
diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py
index 610aa4cc..c2e77365 100755
--- a/test/functional/test_framework/p2p.py
+++ b/test/functional/test_framework/p2p.py
@@ -900,13 +900,12 @@ class P2PDataStore(P2PInterface):
else:
assert_not_equal(node.getbestblockhash(), blocks[-1].hash_hex)
- 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:
@@ -918,10 +917,7 @@ class P2PDataStore(P2PInterface):
for tx in txs:
self.send_without_ping(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:
Why this scored 14/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.