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

Update HandshakeHandler.scala #230

Closed
wants to merge 2 commits into from
Closed

Conversation

faddat
Copy link
Contributor

@faddat faddat commented Sep 9, 2020

minimum supported version is now 0.3.0

PR Details

This PR ensures that old nodes do not connect to new nodes.

Motivation and Context

This will fix node synchronization issues.

How Has This Been Tested

I compiled it on my raspberry pi and ran it and the raspberry pi was able to successfully sync.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

minimum supported version is now 0.3.0
@faddat
Copy link
Contributor Author

faddat commented Sep 9, 2020

will stay draft until a few more checks have been run.

@faddat
Copy link
Contributor Author

faddat commented Sep 9, 2020

closes #194

@faddat
Copy link
Contributor Author

faddat commented Sep 9, 2020

@ncying

Initially this looked like it had solve the problem, but it does not seem to. Could you please audit my changes?

To create these changes and I based my approach on what I saw in the waves repository:

https://github.com/wavesplatform/Waves/blob/d419752eeef9776ce46cfbd12733362c91cc5163/node/src/main/scala/com/wavesplatform/network/HandshakeHandler.scala#L146

my changes were intended to say: "connect to nothing with a version lower than 0.3.0

But after maybe 15 minutes of initial success where I saw it connect to and then disconnect from the nasty node, it Reverted to failure.

@faddat
Copy link
Contributor Author

faddat commented Sep 9, 2020

Screen Shot 2020-09-09 at 4 33 38 PM

@faddat faddat requested a review from ncying September 10, 2020 17:56
Trying a waves-style range of versions to see if it will make us reject as we should.
@faddat faddat marked this pull request as ready for review September 10, 2020 19:03
@faddat
Copy link
Contributor Author

faddat commented Sep 10, 2020

The latest changes seem to resolve the issue.

I guess it needed a range of versions to work properly.

@faddat
Copy link
Contributor Author

faddat commented Sep 11, 2020

Or not

Screenshot_20200911-084846

@faddat faddat mentioned this pull request Sep 11, 2020
@faddat faddat marked this pull request as draft February 25, 2021 02:00
@faddat
Copy link
Contributor Author

faddat commented Mar 1, 2021

Closing, because this did not seem to be an effective solution to issues establishing an initial sync.

@faddat faddat closed this Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant