test: replace (send_message + sync_with_ping) with send_and_ping #18494

This is a follow-up to faf1d047313e71658fb31f6b94fdd5d37705ab85 yesterday.

Concept ACK Note that in p2p_filter.py you could also tackle the sending-and-pinging of msg_filterclear in the last subtest. I think there are still many other occurences of this pattern in the functional tests (I saw them in at least 5-6 different tests recently as far as I remember), but as long as a PR approaches at least all per certain files that's ok I guess.

utACK 7e30eb7

@theStack @vasild thank you -- updated with @theStack's suggestion as per git diff 7e30eb7 5cd2f20. Re-checking to add any others.

re-utACK 5cd2f204d9028c609b61e523937d85c7aa8f8d12

Thanks again @vasild. Added the remaining ones (that make sense to change): git diff 7e30eb7 6112a20

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #18498 (test: enable opening partial p2p connections where useful by jonatack)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

utACK 6112a20

You almost catched them all now, after grepping and inspecting the functional test I found three remaining occurences: https://github.com/bitcoin/bitcoin/blob/6bdd515ccf1b0d3ec5990467db87b9f7963b9b51/test/functional/p2p_segwit.py#L128-L129 https://github.com/bitcoin/bitcoin/blob/6bdd515ccf1b0d3ec5990467db87b9f7963b9b51/test/functional/p2p_segwit.py#L140-L141 https://github.com/bitcoin/bitcoin/blob/6bdd515ccf1b0d3ec5990467db87b9f7963b9b51/test/functional/p2p_leak_tx.py#L42-L43

@theStack thanks for verifying! I don't think the first two cases work, and the last one is already added unless I'm confused.

@jonatack: Oops, you are right, for some reason I overlooked that the change in p2p_leak_tx.py was already part of your commit. I don't see a reason why it wouldn't work for the first two cases though. Given that node is an instance of the class P2PInterface, the snippet

node.send_message(foobar);
node.sync_with_ping();

should behave exactly the same as

node.send_and_ping(foobar);

no matter how complex the expression foobar is. Or am I missing something here?

Can't this be done with a scripted diff?

perl -0777 -i -pe 's/\n([a-z A-Z\._]+)\.send_message\(([^\n]+)\)\n\1\.sync_with_ping\(\)\n/\n$1.send_and_ping($2)\n/gs'

Thanks for reviewing. There are cases where it doesn't make sense, or where it breaks the test, to make the change, and others that wouldn't be seen. I think this now covers the correct ones and is ready.

ACK 6112a209828c43930f677c45461339cdf68a56e9 🎞

<details><summary>Show signature and timestamp</summary>

Signature:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 6112a209828c43930f677c45461339cdf68a56e9 🎞
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUiYjwv9FE2tAKgeRs4uwRH/7SYYLQ/70VCkJzKFQbcZ92s44UF063cXkrff13Ey
l4vyJzOa/Nh88t/+L/2SfRrndkTBZkv6zVixK1FP5RQH/TezHIZK2Odse3Pyu78I
CcyFYQZ0vKCyMqFRIReoSgiWbcKDMMpcFY3NNNtT1WBjIhkd+n/3b+P5BcjFTYj0
m9cZJ9Bw68xNxBVWduwnq/E5YbmXhE8XXpAEAOPHB9XasHfQmV3k39r4txQsBL/6
Vt6REPiv4MwNjby/xEYiqikZhYF1cvla+Rgby56Fe7b0sLDO+c95BJvqOSUL1Vt9
ayi0X1sgN7VFrExHtEBtcnNECs9pedYBzP7kKUgQeENV2hdhwy5PGFOYTRxeor2l
2HOEVbCaff1oYzq/gWNbMH3M9e4c41P347sXqFLYqtIbGWU4QFZApTZ+1D0YfCta
R7NxrDHmmIyQy9ch6po44aJ+PO9T06b//qI1L78i20jcGegQQq2btSlaTqjBujns
D/YSdpwU
=BVs9
-----END PGP SIGNATURE-----

Timestamp of file with hash bba67858ba7d762e61143110f42d94eddd837c955e06827a1e1de70e2c30db8c -

</details>

btw, I ran with the ones provided by @theStack in #18494#pullrequestreview-386418913 and the tests passed

same... I suspect I ran them with send_with_ping by mistake