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

Revert changes to INIT_PROTO_VERSION #2473

Merged
merged 2 commits into from Jun 24, 2017

Conversation

Projects
None yet
5 participants
@ebfull
Copy link
Contributor

commented Jun 24, 2017

This reverts #2245 in order to fix degraded networking behavior for 1.0.10 clients.

@bitcartel

This comment has been minimized.

Copy link
Contributor

commented Jun 24, 2017

ACK

@ebfull

This comment has been minimized.

Copy link
Contributor Author

commented Jun 24, 2017

@zkbot r+

@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Jun 24, 2017

📌 Commit 15a9937 has been approved by ebfull

@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Jun 24, 2017

⌛️ Testing commit 15a9937 with merge 1605da7...

zkbot added a commit that referenced this pull request Jun 24, 2017

Auto merge of #2473 - ebfull:init-proto-hotfix-release, r=ebfull
Revert changes to INIT_PROTO_VERSION

This reverts #2245 in order to fix degraded networking behavior for 1.0.10 clients.
@str4d

str4d approved these changes Jun 24, 2017

Copy link
Contributor

left a comment

ACK.

@@ -12,9 +12,26 @@
static const int PROTOCOL_VERSION = 170002;

//! initial proto version, to be increased after version/verack negotiation
static const int INIT_PROTO_VERSION = 170002;
static const int INIT_PROTO_VERSION = 209;

This comment has been minimized.

Copy link
@str4d

str4d Jun 24, 2017

Contributor

I believe this is the only revert we actually require, but agree with being conservative and reverting the entire PR.

This comment has been minimized.

Copy link
@str4d

str4d Jun 24, 2017

Contributor

I just tried reverting solely this change, and I still encounter degraded network behaviour. So ACK on reverting the entire PR.

@daira

daira approved these changes Jun 24, 2017

Copy link
Contributor

left a comment

utACK, but someone should double check that this resolves the problem.

@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Jun 24, 2017

☀️ Test successful - pr-merge
Approved by: ebfull
Pushing 1605da7 to master...

@zkbot zkbot merged commit 15a9937 into zcash:master Jun 24, 2017

1 check passed

homu Test successful
Details
@@ -103,7 +103,8 @@ class CAddress : public CService
Init();
if (nType & SER_DISK)
READWRITE(nVersion);
if ((nType & SER_DISK) || !(nType & SER_GETHASH))
if ((nType & SER_DISK) ||
(nVersion >= CADDR_TIME_VERSION && !(nType & SER_GETHASH)))

This comment has been minimized.

Copy link
@daira

daira Jun 24, 2017

Contributor

This reversion is also necessary. nVersion here can refer either to INIT_PROTO_VERSION (when deserializing the address fields in the initial "version" message received from a peer), or to the final negotiated version (when deserializing later messages).

@daira daira added this to the 1.0.10-1 Release milestone Jun 24, 2017

@daira daira added this to Blocked in Security and Stability Jul 3, 2017

@daira daira moved this from Blocked to Complete in Security and Stability Jul 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.