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

After network upgrade activation, close existing connections to outdated peers #2262

Closed
8 tasks
teor2345 opened this issue Jun 8, 2021 · 1 comment · Fixed by #3108
Closed
8 tasks

After network upgrade activation, close existing connections to outdated peers #2262

teor2345 opened this issue Jun 8, 2021 · 1 comment · Fixed by #3108
Assignees
Labels
A-network Area: Network protocol updates or fixes A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement C-security Category: Security issues I-slow Problems with performance or responsiveness NU-0 Overwinter Network Upgrade: Overwinter specific tasks (Sprout after Overwinter)

Comments

@teor2345
Copy link
Contributor

teor2345 commented Jun 8, 2021

Motivation

zcashd automatically disconnects from outdated peers after a network upgrade activates.

Specifications

Original specification: ZIP 201: Network Peer Management for Overwinter

Since this is a network protocol specification, Zebra is free to adopt any compatible design.

Draft Zebra behaviour: ZIP 252: Deployment of the NU5 Network Upgrade - Support in Zebra

Solution

Peer Set:

  • Add a ReadyService type containing a Discover::Service peer service
  • Track the service connection's network protocol version in the ReadyService and UnreadyService
  • Close any open connections from outdated peers when the min_peer_version increases
    • security: rate-limit these connection closes to 1 per second, to avoid connection floods and temporary starvation
      • this rate-limit can be implemented in PeerSet::poll_ready

Documentation:

Testing:

  • Sync past the testnet activation height, and check Zebra's behaviour
  • Run and monitor Zebra nodes on mainnet during activation
  • TODO: what other tests can we do here, without too much effort?

Alternatives

We could rely on natural connection churn to get rid of these peers. But it doesn't provide any guarantees.

We could do #2813 after NU5 mainnet activation, but it would only apply to updated Zebra instances.

Related Work

Depends on #1334: Automatically reject new connections from outdated peers after network upgrade activation

@teor2345 teor2345 added A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code NU-0 Overwinter Network Upgrade: Overwinter specific tasks (Sprout after Overwinter) C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage P-Medium C-security Category: Security issues I-slow Problems with performance or responsiveness I-remote-node-overload Zebra can overload other nodes on the network A-network Area: Network protocol updates or fixes labels Jun 8, 2021
@teor2345 teor2345 added this to the 2021 Sprint 13 milestone Jun 8, 2021
@teor2345
Copy link
Contributor Author

The AddressBook-based design was wrong, because the address book only tracks outbound peer connections.

I replaced it with a peer-set based design.

@teor2345 teor2345 self-assigned this Jun 14, 2021
@teor2345 teor2345 changed the title Automatically disconnect from outdated peers after network upgrade activation After network upgrade activation, close existing connections to outdated peers Jun 14, 2021
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Jun 17, 2021
@mpguerra mpguerra linked a pull request Jun 29, 2021 that will close this issue
3 tasks
@mpguerra mpguerra added this to the 2021 Sprint 14 milestone Jul 12, 2021
@mpguerra mpguerra removed this from the 2021 Sprint 21 milestone Sep 30, 2021
@teor2345 teor2345 removed A-consensus Area: Consensus rule updates I-remote-node-overload Zebra can overload other nodes on the network labels Nov 12, 2021
@mpguerra mpguerra linked a pull request Nov 29, 2021 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-network Area: Network protocol updates or fixes A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement C-security Category: Security issues I-slow Problems with performance or responsiveness NU-0 Overwinter Network Upgrade: Overwinter specific tasks (Sprout after Overwinter)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants