test: fix check SENDTXRCNCL without WTXIDRELAY is ignored in `p2p_sendtxrcncl.py` #30683

pull brunoerg wants to merge 1 commits into bitcoin:master from brunoerg:2024-08-test-p2p-sendtxrcncl changing 1 files +1 −1

brunoerg commented at 12:13 PM on August 20, 2024: contributor

Sending a verack without wtxidrelay, regardless of sendtxrcncl, will trigger Forget txreconciliation state of peer. To ensure that we are testing that SENDTXRCNCL without WTXIDRELAY is ignored, we can first verify that the peer was registered.
test: fix check SENDTXRCNCL without WTXIDRELAY is ignored in `p2p_sendtxrcncl.py` 288626a374
DrahtBot commented at 12:13 PM on August 20, 2024: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Code Coverage

For detailed information about the code coverage, see the test coverage report.



Reviews

See the guideline for information on the review process. A summary of reviews will appear here.
DrahtBot added the label Tests on Aug 20, 2024

brunoerg commented at 12:15 PM on August 20, 2024: contributor

Can be tested with:

diff --git a/test/functional/p2p_sendtxrcncl.py b/test/functional/p2p_sendtxrcncl.py
index 2c7216b5ca..1e0e82f076 100755
--- a/test/functional/p2p_sendtxrcncl.py
+++ b/test/functional/p2p_sendtxrcncl.py
@@ -218,7 +218,6 @@ class SendTxRcnclTest(BitcoinTestFramework):
         self.log.info('SENDTXRCNCL without WTXIDRELAY is ignored (recon state is erased after VERACK)')
         peer = self.nodes[0].add_p2p_connection(PeerNoVerack(wtxidrelay=False), send_version=True, wait_for_verack=False)
         with self.nodes[0].assert_debug_log(['Forget txreconciliation state of peer']):
-            peer.send_message(create_sendtxrcncl_msg())
             peer.send_message(msg_verack())
         self.nodes[0].disconnect_p2ps()

in test/functional/p2p_sendtxrcncl.py:220 in 288626a374
```
 216 | @@ -217,7 +217,7 @@ def run_test(self):
 217 |  
 218 |          self.log.info('SENDTXRCNCL without WTXIDRELAY is ignored (recon state is erased after VERACK)')
 219 |          peer = self.nodes[0].add_p2p_connection(PeerNoVerack(wtxidrelay=False), send_version=True, wait_for_verack=False)
 220 | -        with self.nodes[0].assert_debug_log(['Forget txreconciliation state of peer']):
 221 | +        with self.nodes[0].assert_debug_log(['Register peer', 'Forget txreconciliation state of peer']):
```
glozow commented at 1:56 PM on August 20, 2024:

Approach nack on using logs to test behavior. Is this covered in unit tests?

brunoerg commented at 2:05 PM on August 20, 2024:

Is this covered in unit tests?

No, it doesn't cover any p2p iteration.

brunoerg commented at 2:18 PM on August 20, 2024:

Tbh, most part of the functional test relies on checking logs which I agree it's a bad way to test behavior. However, since we're just registering and forgetting peers, there is no other way to test stuff on the functional side, except disconnections.

I'll close this for now and suggest some improvements for this test according to the Erlay PRs.
brunoerg commented at 2:19 PM on August 20, 2024: contributor

#30683 (review)
brunoerg closed this on Aug 20, 2024
bitcoin locked this on Aug 20, 2025

Contributors

Labels

test: fix check SENDTXRCNCL without WTXIDRELAY is ignored in `p2p_sendtxrcncl.py` #30683

Code Coverage

Reviews