Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

P2P 'alert' messaging was removed, updating DevDocs (also updating protocol table to 70015) #1576

Merged
merged 1 commit into from May 14, 2017

Conversation

Projects
None yet
6 participants
Contributor

jonathancross commented Apr 23, 2017

Now that v0.14.0 has shipped, it seemed like a good idea to update this info.
Suggestions / modifications welcome!

Most of this info was gathered from:

_includes/devdoc/guide_p2p_network.md
-
-**Resource:** More details about the structure of messages and a complete list of message types can be found in
-the [P2P reference section][section P2P reference].
+Earlier versions of Bitcoin Core allowed developers and trusted community members to issue [Bitcoin alerts](https://bitcoin.org/en/alerts) to notify users of critical network-wide issues. This messaging system [was retired](https://bitcoin.org/en/alert/2016-11-01-alert-retirement) in Bitcoin Core v0.14.0; however, internal alerts, partition detection warnings and the `-alertnotify` option features remain.
@jonathancross

jonathancross Apr 23, 2017

Contributor

This does not have to stay.
I'm perfectly happy being more aggressive and removing the entire "Alerts" section.
We still note the removal in the P2P network protocol table below which might be enough.

_includes/devdoc/ref_p2p_networking.md
-The `alert` message warns nodes of problems that may affect them or the
@jonathancross

jonathancross Apr 23, 2017

Contributor

I am not familiar with "internal alerts" mentioned by @btcdrak in bitcoin/bitcoin#7692, is any of this deleted material relevant now?

@wbnns wbnns self-assigned this Apr 24, 2017

Contributor

jonathancross commented Apr 24, 2017

I was incorrect about the Alert system being removed in 0.14.0.
It was removed in 0.13.0.

This has now been corrected.

Contributor

laanwj commented Apr 26, 2017

Good riddance.
ACK

Contributor

jonathancross commented Apr 26, 2017

@laanwj Can you assist with #1577 ? Would be nice to update info on the alert key public release.

Contributor

instagibbs commented May 4, 2017

ACK

@@ -64,10 +63,11 @@ with the most recent versions listed first. (If you know of a protocol
version that implemented a major change but which is not listed here,
please [open an issue][docs issue].)
-As of Bitcoin Core 0.13.0, the most recent protocol version is 70014.
+As of Bitcoin Core 0.14.1, the most recent protocol version is 70015.
@achow101

achow101 May 8, 2017

Contributor

I think a description of the 70015 changes should be included in the table below.

@jonathancross

jonathancross May 9, 2017

Contributor

Done.

_includes/devdoc/ref_p2p_networking.md
| Version | Initial Release | Major Changes
|---------|------------------------------------|--------------
+| 70014 | Bitcoin Core 0.13.0 <br> | • Removed `alert` message system. See [Alert System Retirement](https://bitcoin.org/en/alert/2016-11-01-alert-retirement).
@achow101

achow101 May 8, 2017

Contributor

This line should be combined with the line below which also describes 70014

@jonathancross

jonathancross May 9, 2017

Contributor

Done.

_includes/devdoc/ref_p2p_networking.md
@@ -693,102 +693,9 @@ d91f4854 ........................... Epoch time: 1414012889
{% autocrossref %}
*Added in protocol version 311.*
+*Removed in Bitcoin Core 0.13.0*
@achow101

achow101 May 8, 2017

Contributor

You should say which protocol version it was removed in too.

@jonathancross

jonathancross May 9, 2017

Contributor

Since alert removal did not get its own protocol number increase, I was not 100% sure what to use here.

The protocol number was 70012 when bitcoin/bitcoin#7692 was merged on Mar 21, 2016. Then the protocol was bumped to 70013 a few hours later for feefilter in #7542, then again to 70014 before v0.13.0 was finally released.

70013 was the first version where the feature did not exist, so I used that version number. Am I understanding that correctly?

New version pushed, please have a look at your convenience.

@@ -64,10 +63,11 @@ with the most recent versions listed first. (If you know of a protocol
version that implemented a major change but which is not listed here,
please [open an issue][docs issue].)
-As of Bitcoin Core 0.13.0, the most recent protocol version is 70014.
+As of Bitcoin Core 0.14.1, the most recent protocol version is 70015.
@jonathancross

jonathancross May 9, 2017

Contributor

Done.

_includes/devdoc/ref_p2p_networking.md
| Version | Initial Release | Major Changes
|---------|------------------------------------|--------------
+| 70014 | Bitcoin Core 0.13.0 <br> | • Removed `alert` message system. See [Alert System Retirement](https://bitcoin.org/en/alert/2016-11-01-alert-retirement).
@jonathancross

jonathancross May 9, 2017

Contributor

Done.

-| 70014 | Bitcoin Core 0.13.0 <br> | [BIP152][]: <br>• Added `sendcmpct`, `cmpctblock`, `getblocktxn`, `blocktxn` messages <br> * Added `MSG_CMPCT_BLOCK` inventory type to `getdata` message.
-| 70013 | Bitcoin Core 0.13.0 <br> | [BIP133][]: <br>• Added `feefilter` message
-| 70012 | Bitcoin Core 0.12.0 <br> | [BIP130][]: <br>• Added `sendheaders` message
+| 70015 | Bitcoin Core 0.13.2 <br> | • New banning behavior for invalid compact blocks [#9026](https://github.com/bitcoin/bitcoin/pull/9026) in v0.14.0, Backported to v0.13.2 in [#9048](https://github.com/bitcoin/bitcoin/pull/9048).
@jonathancross

jonathancross May 9, 2017

Contributor

The protocol bump to 70015 was made in v0.14.0 (#9026), then backported to v0.13.2 in #9048.
Considering there was no clear announcement in release notes, I opted for this description with references to pull requests. Happy to change it if needed.

_includes/devdoc/ref_p2p_networking.md
@@ -693,102 +693,9 @@ d91f4854 ........................... Epoch time: 1414012889
{% autocrossref %}
*Added in protocol version 311.*
+*Removed in Bitcoin Core 0.13.0*
@jonathancross

jonathancross May 9, 2017

Contributor

Since alert removal did not get its own protocol number increase, I was not 100% sure what to use here.

The protocol number was 70012 when bitcoin/bitcoin#7692 was merged on Mar 21, 2016. Then the protocol was bumped to 70013 a few hours later for feefilter in #7542, then again to 70014 before v0.13.0 was finally released.

70013 was the first version where the feature did not exist, so I used that version number. Am I understanding that correctly?

_includes/devdoc/ref_p2p_networking.md
@@ -693,102 +693,9 @@ d91f4854 ........................... Epoch time: 1414012889
{% autocrossref %}
*Added in protocol version 311.*
+*Removed in protocol version 70013 and released in Bitcoin Core v0.13.0*
@jonathancross

jonathancross May 9, 2017

Contributor

Since alert removal did not get its own protocol number increase, I was not 100% sure what to use here.

The protocol number was 70012 when bitcoin/bitcoin#7692 was merged on Mar 21, 2016. Then the protocol was bumped to 70013 a few hours later for feefilter in #7542, then again to 70014 before v0.13.0 was finally released.

70013 was the first version where the feature did not exist, so I used that version number. Am I understanding that correctly?

@harding

harding May 10, 2017

Contributor

I think that's reasonable.

As a nitpick, could you remove the v in v0.13.0? That will allow the auto-linker to link Bitcoin Core 0.13.0 to the release notes for that version as it does in the protocol versions table:

2017-05-10-05_17_58_254867513

@jonathancross

jonathancross May 10, 2017

Contributor

No problem :-) Done.

@jonathancross jonathancross changed the title from P2P 'alert' messaging was removed, updating DevDocs to P2P 'alert' messaging was removed, updating DevDocs (also updating protocol table to 70015) May 9, 2017

One tiny nitpick but otherwise tested ACK. Thanks, @jonathancross!

_includes/devdoc/ref_p2p_networking.md
@@ -693,102 +693,9 @@ d91f4854 ........................... Epoch time: 1414012889
{% autocrossref %}
*Added in protocol version 311.*
+*Removed in protocol version 70013 and released in Bitcoin Core v0.13.0*
@harding

harding May 10, 2017

Contributor

I think that's reasonable.

As a nitpick, could you remove the v in v0.13.0? That will allow the auto-linker to link Bitcoin Core 0.13.0 to the release notes for that version as it does in the protocol versions table:

2017-05-10-05_17_58_254867513

Contributor

jonathancross commented May 10, 2017

Thanks @harding, should be good to go now.

Contributor

achow101 commented May 12, 2017

LGTM

Contributor

harding commented May 12, 2017

8d84279 tested ACK. Thanks!

Contributor

wbnns commented May 13, 2017

Thanks everybody - unless others object, this will be merged on Sunday, May 14th (tomorrow).

@wbnns wbnns merged commit 822947a into bitcoin-dot-org:master May 14, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Contributor

jonathancross commented May 15, 2017

Cheers, thanks everyone!

@jonathancross jonathancross deleted the jonathancross:alert branch May 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment