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

refactor(sync): refactoring syncing process #676

Merged
merged 4 commits into from
Sep 7, 2023

Conversation

b00f
Copy link
Collaborator

@b00f b00f commented Sep 5, 2023

Description

This PR includes these changes:

1- HeartBeat message is removed.
2- HelloAck message is added. Once a node receives a HelloAck, it can start sending messages.
3- MaxOpenSessions is removed and the maximum number of sessions should be the same as the number of connected peers.
4- Syncing process is based on time, not height. If the last block time is more than 20 seconds behind the current time, the node starts syncing.

@b00f b00f requested review from ZigBalthazar, amirvalhalla and kehiy and removed request for ZigBalthazar September 5, 2023 16:47
@codecov
Copy link

codecov bot commented Sep 5, 2023

Codecov Report

Merging #676 (665a4c3) into main (4513a1d) will decrease coverage by 0.21%.
The diff coverage is 88.57%.

@@            Coverage Diff             @@
##             main     #676      +/-   ##
==========================================
- Coverage   83.70%   83.50%   -0.21%     
==========================================
  Files         167      168       +1     
  Lines        8009     7922      -87     
==========================================
- Hits         6704     6615      -89     
- Misses       1002     1004       +2     
  Partials      303      303              

Copy link
Contributor

@kehiy kehiy left a comment

Choose a reason for hiding this comment

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

LGTM.
That's exactly what we discussed!

Copy link
Member

@amirvalhalla amirvalhalla left a comment

Choose a reason for hiding this comment

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

@b00f
please resolve request changes.

@amirvalhalla
Copy link
Member

@b00f
I've approved the PR but still you didn't resolve my request changes

@b00f
Copy link
Collaborator Author

b00f commented Sep 6, 2023

@amirvalhalla could you please double check it. If anything missed, mention me.

Copy link
Member

@amirvalhalla amirvalhalla left a comment

Choose a reason for hiding this comment

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

@b00f
please resolve request changes.

@themantre themantre removed the request for review from ZigBalthazar September 7, 2023 14:38
@themantre themantre merged commit 6c53a36 into pactus-project:main Sep 7, 2023
12 checks passed
@b00f b00f deleted the refactor-sync branch September 7, 2023 14:50
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.

4 participants