-
Notifications
You must be signed in to change notification settings - Fork 38.3k
test: address self-announcement #34039
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
test: address self-announcement #34039
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34039. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please copy-paste |
| # m_next_local_addr_send and AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL: | ||
| # self-announcements are sent on an exponential distribution with mean interval of 24h. | ||
| # Setting the mocktime 20d forward gives a probability of (1 - e^-(480/24)) that | ||
| # the event will occur (i.e. this fails once in ~500 million repeats). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inspired by
bitcoin/test/functional/p2p_addr_relay.py
Lines 130 to 133 in cca113f
| # invoke m_next_addr_send timer: | |
| # `addr` messages are sent on an exponential distribution with mean interval of 30s. | |
| # Setting the mocktime 600s forward gives a probability of (1 - e^-(600/30)) that | |
| # the event will occur (i.e. this fails once in ~500 million repeats). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nothing for this PR, but since these things are quite common (recently #33121), I wonder if a mode -test=cutoffexponentialtimers or something similar would make sense which would cut off the long tail of all exponential timers (rand_exp_duration) at some percentile, so we don't need to deal with these probabilistic failures in functional tests.
Test that a node sends a self-announcement with its external IP to in- and outbound peers after connection open and again sometime later. Since the code for the test is mostly the same for addr and addrv2 messages, I opted to add a new test file instead of having duplicate code in p2p_addr_relay.py and p2p_addrv2_relay.py.
2cac7e0 to
fb9c393
Compare
| first_octet = 1 + (i >> 8) | ||
| second_octet = i % 256 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since increasing the 50 in the loop will not contribute anything to the test, maybe the >> 8 and % 256 could be erased.
| first_octet = 1 + (i >> 8) | |
| second_octet = i % 256 | |
| first_octet = 1 + i | |
| second_octet = i |
|
Code review ACK fb9c393. I have run the test and everything checks out. I left a small comment in the code, also fine with merging as-is |
Test that a node sends a self-announcement with its external IP to in- and outbound peers after connection open and again sometime later.
Since the code for the test is mostly the same for addr and addrv2 messages, I opted to add a new test file instead of having duplicate code in
p2p_addr_relay.pyandp2p_addrv2_relay.py.