Skip to content
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

[tests] Dead mininode code #11638

Merged
merged 4 commits into from Nov 9, 2017
Merged

Conversation

jnewbery
Copy link
Contributor

@jnewbery jnewbery commented Nov 8, 2017

This is the first part of #11518. It removes a ~150 lines of unused code from the mininode module:

  • remove unused deliver_sleep_time and EarlyDisconnectError code
  • remove support for pre-BIP31 ping messages
  • remove support for alert message
  • explicitly don't support p2p versions lower than 60001

Should be an easy ACK for reviewers. If all extended tests pass, then this code really was dead :)

Removes the dead deliver_sleep_time and EarlyDisconnectError code
BIP31 support was added to Bitcoin Core in version 0.6.1. Our test
framework is incompatible with Bitcoin Core versions that old, so remove
all special logic for handling pre-BIP31 pings.
Alert messages were removed in p2p version 70013 (Bitcoin Core V0.13.0)
The mininode module includes code to support p2p versions below
60001. However, the test_framework does not support versions
of Bitcoin Core before V0.13.0. Remove code supporting
p2p versions before 60001 (which has never been run).
Copy link
Member

@jamesob jamesob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK fb00c45

Grep's happy with the removal:

(venv)  [Wed 8 20:42] james/tmp/bitcoin fb00c45c3* 
 $ for i in on_alert BIP0031_VERSION CUnisgnedAlert CAlert msg_alert msg_ping_prebip31 get_deliver_sleep_time EarlyDisconnectError; do git grep $i | cat; done

src/net.cpp:                else if (nTime - pnode->nLastRecv > (pnode->nVersion > BIP0031_VERSION ? TIMEOUT_INTERVAL : 90*60))
src/net_processing.cpp:        if (pfrom->nVersion > BIP0031_VERSION)
src/net_processing.cpp:            if (pto->nVersion > BIP0031_VERSION) {
src/version.h:static const int BIP0031_VERSION = 60000;
doc/developer-notes.md:class CAlert

@practicalswift
Copy link
Contributor

Very nice cleanup!

utACK fb00c45

@laanwj
Copy link
Member

laanwj commented Nov 9, 2017

utACK fb00c45

(there's an argument to leave support for older versions in the mininode framework: we might want to test how the code interacts with older versions. But 60001 as bottom is fine, apparently that was introduced in 0.6.2, that's ancient history)

@jnewbery
Copy link
Contributor Author

jnewbery commented Nov 9, 2017

(there's an argument to leave support for older versions in the mininode framework: we might want to test how the code interacts with older versions. But 60001 as bottom is fine, apparently that was introduced in 0.6.2, that's ancient history)

Exactly. See the commit message for c0b1274:

BIP31 support was added to Bitcoin Core in version 0.6.1. Our test
framework is incompatible with Bitcoin Core versions that old, so remove
all special logic for handling pre-BIP31 pings.

@maflcko
Copy link
Member

maflcko commented Nov 9, 2017

utACK fb00c45

@maflcko maflcko merged commit fb00c45 into bitcoin:master Nov 9, 2017
maflcko pushed a commit that referenced this pull request Nov 9, 2017
fb00c45 [tests] Explicitly disallow support for p2p versions below 60001 (John Newbery)
3858aab [tests] Remove support for p2p alert messages (John Newbery)
c0b1274 [tests] Remove support for bre-BIP31 ping messages (John Newbery)
2904e30 [tests] Remove dead code from mininode.py (John Newbery)

Pull request description:

  This is the first part of #11518. It removes a ~150 lines of unused code from the mininode module:

  - remove unused `deliver_sleep_time` and `EarlyDisconnectError` code
  - remove support for pre-BIP31 ping messages
  - remove support for alert message
  - explicitly don't support p2p versions lower than 60001

  Should be an easy ACK for reviewers. If all extended tests pass, then this code really was dead :)

Tree-SHA512: 508e612ceb0b094250d18e75522d51e6b14cd069443050ba4af34d6f890c58721cb5653e8bc000b60635b9474d035b0dcd9c509c0dcdb3a7501df17b787f83b0
@jnewbery jnewbery deleted the dead_mininode_code branch November 9, 2017 19:42
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 11, 2019
fb00c45 [tests] Explicitly disallow support for p2p versions below 60001 (John Newbery)
3858aab [tests] Remove support for p2p alert messages (John Newbery)
c0b1274 [tests] Remove support for bre-BIP31 ping messages (John Newbery)
2904e30 [tests] Remove dead code from mininode.py (John Newbery)

Pull request description:

  This is the first part of bitcoin#11518. It removes a ~150 lines of unused code from the mininode module:

  - remove unused `deliver_sleep_time` and `EarlyDisconnectError` code
  - remove support for pre-BIP31 ping messages
  - remove support for alert message
  - explicitly don't support p2p versions lower than 60001

  Should be an easy ACK for reviewers. If all extended tests pass, then this code really was dead :)

Tree-SHA512: 508e612ceb0b094250d18e75522d51e6b14cd069443050ba4af34d6f890c58721cb5653e8bc000b60635b9474d035b0dcd9c509c0dcdb3a7501df17b787f83b0
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 13, 2020
fb00c45 [tests] Explicitly disallow support for p2p versions below 60001 (John Newbery)
3858aab [tests] Remove support for p2p alert messages (John Newbery)
c0b1274 [tests] Remove support for bre-BIP31 ping messages (John Newbery)
2904e30 [tests] Remove dead code from mininode.py (John Newbery)

Pull request description:

  This is the first part of bitcoin#11518. It removes a ~150 lines of unused code from the mininode module:

  - remove unused `deliver_sleep_time` and `EarlyDisconnectError` code
  - remove support for pre-BIP31 ping messages
  - remove support for alert message
  - explicitly don't support p2p versions lower than 60001

  Should be an easy ACK for reviewers. If all extended tests pass, then this code really was dead :)

Tree-SHA512: 508e612ceb0b094250d18e75522d51e6b14cd069443050ba4af34d6f890c58721cb5653e8bc000b60635b9474d035b0dcd9c509c0dcdb3a7501df17b787f83b0
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 13, 2020
fb00c45 [tests] Explicitly disallow support for p2p versions below 60001 (John Newbery)
3858aab [tests] Remove support for p2p alert messages (John Newbery)
c0b1274 [tests] Remove support for bre-BIP31 ping messages (John Newbery)
2904e30 [tests] Remove dead code from mininode.py (John Newbery)

Pull request description:

  This is the first part of bitcoin#11518. It removes a ~150 lines of unused code from the mininode module:

  - remove unused `deliver_sleep_time` and `EarlyDisconnectError` code
  - remove support for pre-BIP31 ping messages
  - remove support for alert message
  - explicitly don't support p2p versions lower than 60001

  Should be an easy ACK for reviewers. If all extended tests pass, then this code really was dead :)

Tree-SHA512: 508e612ceb0b094250d18e75522d51e6b14cd069443050ba4af34d6f890c58721cb5653e8bc000b60635b9474d035b0dcd9c509c0dcdb3a7501df17b787f83b0

readd is None check

Signed-off-by: Pasta <pasta@dashboost.org>
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 13, 2020
fb00c45 [tests] Explicitly disallow support for p2p versions below 60001 (John Newbery)
3858aab [tests] Remove support for p2p alert messages (John Newbery)
c0b1274 [tests] Remove support for bre-BIP31 ping messages (John Newbery)
2904e30 [tests] Remove dead code from mininode.py (John Newbery)

Pull request description:

  This is the first part of bitcoin#11518. It removes a ~150 lines of unused code from the mininode module:

  - remove unused `deliver_sleep_time` and `EarlyDisconnectError` code
  - remove support for pre-BIP31 ping messages
  - remove support for alert message
  - explicitly don't support p2p versions lower than 60001

  Should be an easy ACK for reviewers. If all extended tests pass, then this code really was dead :)

Tree-SHA512: 508e612ceb0b094250d18e75522d51e6b14cd069443050ba4af34d6f890c58721cb5653e8bc000b60635b9474d035b0dcd9c509c0dcdb3a7501df17b787f83b0

readd is None check

Signed-off-by: Pasta <pasta@dashboost.org>
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 29, 2020
fb00c45 [tests] Explicitly disallow support for p2p versions below 60001 (John Newbery)
3858aab [tests] Remove support for p2p alert messages (John Newbery)
c0b1274 [tests] Remove support for bre-BIP31 ping messages (John Newbery)
2904e30 [tests] Remove dead code from mininode.py (John Newbery)

Pull request description:

  This is the first part of bitcoin#11518. It removes a ~150 lines of unused code from the mininode module:

  - remove unused `deliver_sleep_time` and `EarlyDisconnectError` code
  - remove support for pre-BIP31 ping messages
  - remove support for alert message
  - explicitly don't support p2p versions lower than 60001

  Should be an easy ACK for reviewers. If all extended tests pass, then this code really was dead :)

Tree-SHA512: 508e612ceb0b094250d18e75522d51e6b14cd069443050ba4af34d6f890c58721cb5653e8bc000b60635b9474d035b0dcd9c509c0dcdb3a7501df17b787f83b0

readd is None check

Signed-off-by: Pasta <pasta@dashboost.org>
gades pushed a commit to cosanta/cosanta-core that referenced this pull request Jun 30, 2021
fb00c45 [tests] Explicitly disallow support for p2p versions below 60001 (John Newbery)
3858aab [tests] Remove support for p2p alert messages (John Newbery)
c0b1274 [tests] Remove support for bre-BIP31 ping messages (John Newbery)
2904e30 [tests] Remove dead code from mininode.py (John Newbery)

Pull request description:

  This is the first part of bitcoin#11518. It removes a ~150 lines of unused code from the mininode module:

  - remove unused `deliver_sleep_time` and `EarlyDisconnectError` code
  - remove support for pre-BIP31 ping messages
  - remove support for alert message
  - explicitly don't support p2p versions lower than 60001

  Should be an easy ACK for reviewers. If all extended tests pass, then this code really was dead :)

Tree-SHA512: 508e612ceb0b094250d18e75522d51e6b14cd069443050ba4af34d6f890c58721cb5653e8bc000b60635b9474d035b0dcd9c509c0dcdb3a7501df17b787f83b0

readd is None check

Signed-off-by: Pasta <pasta@dashboost.org>
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants