[tests] Test disconnecting unsupported service bits logic. #11001
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In v0.15, we disconnect nodes that send us version messages with unsupported service bits (1 << 5 and 1 << 7). This commit adds a test that bitcoind will disconnect those nodes before August 1st 2018, and won't disconnect those nodes after August 1st 2018.
Contributor
Author
Contributor
|
Thanks, looks good to me.
…On August 7, 2017 8:41:21 AM PDT, John Newbery ***@***.***> wrote:
In v0.15, we disconnect nodes that send us version messages with
unsupported service bits (1 << 5 and 1 << 7). This commit adds a test
that bitcoind will disconnect those nodes before August 1st 2018, and
won't
disconnect those nodes after August 1st 2018.
@sdaftuar @TheBlueMatt
You can view, comment on, or merge this pull request online at:
#11001
-- Commit Summary --
* [tests] Test disconnecting unsupported service bits logic.
-- File Changes --
M test/functional/p2p-leaktests.py (34)
M test/functional/test_framework/mininode.py (2)
-- Patch Links --
https://github.com/bitcoin/bitcoin/pull/11001.patch
https://github.com/bitcoin/bitcoin/pull/11001.diff
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#11001
|
Member
|
utACK 5e35cd9 |
maflcko
pushed a commit
that referenced
this pull request
Aug 7, 2017
5e35cd9 [tests] Test disconnecting unsupported service bits logic. (John Newbery) Pull request description: In v0.15, we disconnect nodes that send us version messages with unsupported service bits (1 << 5 and 1 << 7). This commit adds a test that bitcoind will disconnect those nodes before August 1st 2018, and won't disconnect those nodes after August 1st 2018. @sdaftuar @TheBlueMatt Tree-SHA512: cb1bf311f9edc4aa5acf621e392543fd92952b2462ae2a8c20339e5c41e67ece1a2a9ede38db5dcd16563faa9ee58318e118f54024dd282c580132202d2df01b
|
Why not relax the rule to instead allow for a maximum of e.g. 3 connections to nodes with the service bits 5 and 7? That way the network may stay fully connected both before and after a 2x HF in three months. |
Member
|
This was meant as a minimal fix/diff to be included in the first release candidate of 0.15.0, which was already due last Sunday. Beside the additional review required for more complex changes to p2p logic, it is unlikely that a more involved fix makes it into 0.15.0. Code cleanup for 0.16.0 is welcome, though. |
Contributor
|
The 2x/Bitcoin networks will stay trivially connected through the next few months. There is no chance that everyone running 0.13/0.14 nodes will upgrade to 15 that quickly - we've never seen upgrades anywhere near that fast in the past, even when there was more incentive to do so.
…On August 9, 2017 4:26:17 AM PDT, bmagn ***@***.***> wrote:
Why not relax the rule to instead allow for a maximum of e.g. 3
connections to nodes with the service bits 5 and 7? That way the
network may stay fully connected both before and after a 2x HF in three
months.
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#11001 (comment)
|
Contributor
Author
|
Please can we lock this PR? This PR added a new functional test case and is not the place to discuss the merits/demerits of the node disconnection behaviour in 10982. Thanks! |
This was referenced Aug 17, 2017
Merged
maflcko
pushed a commit
that referenced
this pull request
Sep 13, 2017
0063d2c [tests] Make p2p-leaktests.py more robust (John Newbery) Pull request description: There has been an example of p2p-leaktests.py failing on travis in the new service bits test (introduced in #11001 . It appeared to me that the previous p2p connections had not been fully disconnected before attempting to add new p2p connections. I've added a sleep and restarted the NetworkThread, but I don't know whether this will fix the problem, since I'm unable to reproduce the failure locally. @MarcoFalke - not sure what you want to do here? I don't think this change could make things any worse. Tree-SHA512: f5427c26267185a903c9b75bb3925bf153b8afce70c8e493bf8f585f57d809d20643b4ee69081300b211d22e960242aecc3d719f4ddd230aa08fdc5484b55055
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Jan 10, 2020
0063d2c [tests] Make p2p-leaktests.py more robust (John Newbery) Pull request description: There has been an example of p2p-leaktests.py failing on travis in the new service bits test (introduced in bitcoin#11001 . It appeared to me that the previous p2p connections had not been fully disconnected before attempting to add new p2p connections. I've added a sleep and restarted the NetworkThread, but I don't know whether this will fix the problem, since I'm unable to reproduce the failure locally. @MarcoFalke - not sure what you want to do here? I don't think this change could make things any worse. Tree-SHA512: f5427c26267185a903c9b75bb3925bf153b8afce70c8e493bf8f585f57d809d20643b4ee69081300b211d22e960242aecc3d719f4ddd230aa08fdc5484b55055
gades
pushed a commit
to cosanta/cosanta-core
that referenced
this pull request
Jun 30, 2021
0063d2c [tests] Make p2p-leaktests.py more robust (John Newbery) Pull request description: There has been an example of p2p-leaktests.py failing on travis in the new service bits test (introduced in bitcoin#11001 . It appeared to me that the previous p2p connections had not been fully disconnected before attempting to add new p2p connections. I've added a sleep and restarted the NetworkThread, but I don't know whether this will fix the problem, since I'm unable to reproduce the failure locally. @MarcoFalke - not sure what you want to do here? I don't think this change could make things any worse. Tree-SHA512: f5427c26267185a903c9b75bb3925bf153b8afce70c8e493bf8f585f57d809d20643b4ee69081300b211d22e960242aecc3d719f4ddd230aa08fdc5484b55055
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In v0.15, we disconnect nodes that send us version messages with
unsupported service bits (1 << 5 and 1 << 7). This commit adds a test
that bitcoind will disconnect those nodes before August 1st 2018, and won't
disconnect those nodes after August 1st 2018.
@sdaftuar @TheBlueMatt