test: check disconnection when sending sendaddrv2 after verack #29699

pull brunoerg wants to merge 1 commits into bitcoin:master from brunoerg:2024-03-test-sendaddrv2-after-verack changing 1 files +9 −2

brunoerg commented at 8:56 PM on March 21, 2024: contributor

This PR adds test coverage for: https://github.com/bitcoin/bitcoin/blob/71b63195b30b2fa0dff20ebb262ce7566dd5d673/src/net_processing.cpp#L3796-L3807
DrahtBot commented at 8:56 PM on March 21, 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.

Type Reviewers

ACK byaye, maflcko

Stale ACK BrandonOdiwuor, kevkevinpal

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.



Conflicts

No conflicts as of last run.
DrahtBot added the label Tests on Mar 21, 2024
maflcko commented at 7:10 AM on March 22, 2024: member

lgtm ACK 8517ee951321e54443c40303791a0e154c2bb81f
BrandonOdiwuor approved
BrandonOdiwuor commented at 12:31 PM on March 25, 2024: contributor

crACK 8517ee951321e54443c40303791a0e154c2bb81f
kevkevinpal commented at 12:55 PM on March 27, 2024: contributor

ACK 8517ee9
in test/functional/p2p_addrv2_relay.py:106 in 8517ee9513 outdated
```
  99 | @@ -99,6 +100,11 @@ def run_test(self):
 100 |          assert addr_receiver.addrv2_received_and_checked
 101 |          assert_equal(len(self.nodes[0].getnodeaddresses(count=0, network="i2p")), 0)
 102 |  
 103 | +        self.log.info('Check disconnection when sending sendaddrv2 after verack')
 104 | +        with self.nodes[0].assert_debug_log(['sendaddrv2 received after verack from peer=0; disconnecting']):
 105 | +            addr_source.send_message(msg_sendaddrv2())
 106 | +            addr_source.wait_for_disconnect()
```
maflcko commented at 6:57 PM on March 27, 2024:

I missed this during review, but it looks like this conflicts with #29575, so it may be better to spin up a new connection and move the code elsewhere to avoid the conflict?

brunoerg commented at 10:04 AM on March 28, 2024:

Yes, I think I can move it to p2p_leak, for example.

maflcko commented at 10:06 AM on March 28, 2024:

You can keep it in the same file. Above self.log.info('Create connection that sends addrv2 messages') should be enough to solve the conflict.

brunoerg commented at 10:22 AM on March 28, 2024:

You're right, will address it.

brunoerg commented at 10:27 AM on March 28, 2024:

Done
brunoerg force-pushed on Mar 28, 2024
test: check disconnection when sending sendaddrv2 after verack b4c9ace6ff
brunoerg force-pushed on Mar 28, 2024
brunoerg commented at 10:27 AM on March 28, 2024: contributor

Force-pushed addressing #29699 (review)
byaye approved
byaye commented at 12:10 PM on April 15, 2024: none

Tested ACK b4c9ace6ff36c54755e4b12f204212c1b938f509

<img width="854" alt="Screenshot 2024-04-15 at 09 03 30" src="https://github.com/bitcoin/bitcoin/assets/19962379/4bc020e1-ecfb-4ac2-88b0-b3cd03cf9a00">
DrahtBot requested review from maflcko on Apr 15, 2024
DrahtBot requested review from BrandonOdiwuor on Apr 15, 2024
maflcko commented at 12:13 PM on April 15, 2024: member

lgtm ACK b4c9ace6ff36c54755e4b12f204212c1b938f509

Type	Reviewers
ACK	byaye, maflcko
Stale ACK	BrandonOdiwuor, kevkevinpal

in test/functional/p2p_addrv2_relay.py:100 in b4c9ace6ff

  95 | @@ -89,8 +96,8 @@ def run_test(self):
  96 |          msg.addrs = ADDRS
  97 |          msg_size = calc_addrv2_msg_size(ADDRS)
  98 |          with self.nodes[0].assert_debug_log([
  99 | -                f'received: addrv2 ({msg_size} bytes) peer=0',
 100 | -                f'sending addrv2 ({msg_size} bytes) peer=1',
 101 | +                f'received: addrv2 ({msg_size} bytes) peer=1',
 102 | +                f'sending addrv2 ({msg_size} bytes) peer=2',