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

[syncer] Forgetting non-Dogechain protocol peer #271

Merged
merged 11 commits into from
Dec 12, 2022

Conversation

DarianShawn
Copy link
Collaborator

Description

Currently, peers will discover and connect to non-Dogechain peers, which is frustrating and extremely draining of connection resources.

The PR fixes it by forgetting (disconnect and remove it from store) the peer which is not supporting Dogechain syncer protocol.

Also, we print out the node ID that is currently syncing in eth_syncing endpoint and update its progress when the node status is updated.

Changes include

  • Bugfix (non-breaking change that solves an issue)
  • New feature (non-breaking change that adds functionality)

Testing

  • I have tested this code with the official test suite
  • I have tested this code manually

Manual tests

  • Start a new MainNet node from Genesis block.
  • Observe its peers protocol list.
    • dogechain peers list | grep "=" | awk '{print $3}' | while read p; do dogechain peers status --peer-id $p; done
  • Observe its log.
  • Observe its syncing status.
    • curl -X POST http://localhost:8545 -d '{"id": 0, "jsonrpc":"2.0", "method": "eth_syncing", "params": []}'

PR branch result:

  • The connected nodes all support the Dogechain syncer protocol.
  • There are some forget peer in the log.
  • Return correct and upgraded progression.

Base branch result:

  • Due to unfiltered protocol reply discovery, connected peers contain some non-Dogechain peers.

  • No and never "forget" any peer in the log.

  • Print out unobtrusive and sometimes erroneous sync status.

  • The connected peers contain some non-Dogechain peers due to unfiltered protocol reply discovery.

  • No and never 'forget' any peer in the log.

  • Progress sometimes go wrong.

Documentation update

Should update official documentation once the version bumped.

@DarianShawn DarianShawn added feature New update to Dogechain bug fix Functionality that fixes a bug labels Dec 12, 2022
@DarianShawn DarianShawn added this to the Release 1.2.2 milestone Dec 12, 2022
@DarianShawn DarianShawn self-assigned this Dec 12, 2022
Copy link
Collaborator

@abrahamcruise321 abrahamcruise321 left a comment

Choose a reason for hiding this comment

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

LGTM.

@abrahamcruise321 abrahamcruise321 merged commit b192cda into dev Dec 12, 2022
@abrahamcruise321 abrahamcruise321 deleted the disconnect-non-dogechain-peer branch December 12, 2022 08:55
@github-actions github-actions bot locked and limited conversation to collaborators Dec 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug fix Functionality that fixes a bug feature New update to Dogechain
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants