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

optimization p2p #1242

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Conversation

lyy4j
Copy link

@lyy4j lyy4j commented Dec 3, 2018

No description provided.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 56.749% when pulling 4e32378 on lyy4j:20181201_p2p_optimization into 5c796de on ethereum:develop.

Copy link
Member

@Nashatyrev Nashatyrev left a comment

Choose a reason for hiding this comment

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

I don't understand what problem is solved with this PR

stateChanged(oldState, newState);
if (newState == State.Active) {
stateChanged(oldState, newState);
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why this change doesn't break anything. Why stateChanged() should be called only on Active?

Copy link
Author

@lyy4j lyy4j Dec 4, 2018

Choose a reason for hiding this comment

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

firstly, when the oldest node challenge With the new node,and the oldest node win,we should update the oldest timestap,or else, the node will be get when a new node was added and the bucket was full at next time.

secondly, as the origin code did ,when a node state was change ,the method stateChanged was called every time ,and the method is mainly to establish tcp connect with the peer node ,did it need ?I think when the node in active status, they should start a connect

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.

3 participants