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, reject new connections from outdated peers #1334

Closed
1 of 8 tasks
teor2345 opened this issue Nov 19, 2020 · 4 comments · Fixed by #2519
Closed
1 of 8 tasks

After network upgrade activation, reject new connections from outdated peers #1334

teor2345 opened this issue Nov 19, 2020 · 4 comments · Fixed by #2519
Assignees
Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-design Category: Software design work C-security Category: Security issues I-heavy Problems with excessive memory, disk, or CPU usage I-slow Problems with performance or responsiveness I-unbounded-growth Zebra keeps using resources, without any limit NU-5 Network Upgrade: NU5 specific tasks

Comments

@teor2345
Copy link
Contributor

teor2345 commented Nov 19, 2020

Motivation

zcashd rejects new connections from outdated peers after a network upgrade activates.

Previously, the Zebra developers manually incremented the CURRENT_VERSION and MIN_NETWORK_UPGRADE version after each network upgrade activation.

But that's not an ideal solution, because it:

  • causes node failures after a network upgrade activation, until the CURRENT_VERSION is incremented
  • keeps connecting to old Testnet peers, until both networks have activated the same MIN_NETWORK_UPGRADE

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

Increment the network protocol version after Zebra's network upgrade implementation is complete (see #1841).

Calculate the minimum peer protocol version based on the current best tip height:

  • Replace MIN_NETWORK_UPGRADE with a min_peer_version function, which changes at each network upgrade activation, based on the best tip height
  • Add a MIN_ACTIVE_NETWORK_UPGRADE function, and use it to calculate the minimum network protocol version we'll accept during the initial sync
    • the function should take a Network argument, and (possibly) return a different network upgrade for each network
    • this function should return Canopy until after NU5 activation on each network
  • If the chain is rolled back past network upgrade activation, allow connections to outdated peers again

Handshaker:

  • Reject new handshakes with outdated peers based on min_peer_version
    • check inbound and outbound peers

Documentation:

Testing:

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

Alternative Solutions

Manually increment MIN_NETWORK_UPGRADE after each network upgrade. This will cause Zebra to fail after each network upgrade, until the user installs the latest version.

Related Issues

A previous PR that incremented these constants is #1333.

#706 evicting old peers in the days before a network upgrade
#337 ZIP-200: Network Upgrade Mechanism

Follow Up

Re-deploy long-running Foundation Zebra nodes

@teor2345 teor2345 added C-design Category: Software design work A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code S-needs-triage Status: A bug report needs triage labels Nov 19, 2020
@teor2345 teor2345 added the NU-5 Network Upgrade: NU5 specific tasks label Nov 19, 2020
@teor2345 teor2345 changed the title Decide how to handle network protocol version upgrades Automatically disconnect old peers after network upgrade activation Nov 19, 2020
@hdevalence
Copy link
Contributor

Rather than implementing any special network upgrade disconnection logic, I think it would be much cleaner to (1) decline to connect to peers speaking the old protocol and (2) ensure that all network connections are short-lived by churning peer connections. This is better because it avoids introducing extra modes into our network code.

@hdevalence
Copy link
Contributor

xref #706 with discussion on the above point

@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Feb 2, 2021
@teor2345 teor2345 added I-slow Problems with performance or responsiveness P-Medium labels Feb 25, 2021
@teor2345 teor2345 added C-security Category: Security issues I-heavy Problems with excessive memory, disk, or CPU usage I-unbounded-growth Zebra keeps using resources, without any limit labels Mar 8, 2021
@teor2345
Copy link
Contributor Author

Assigning this ticket to sprint 7, because it's required for testnet activation.

@teor2345 teor2345 changed the title Automatically disconnect old peers after network upgrade activation Automatically reject connections from outdated peers after network upgrade activation Jun 8, 2021
@teor2345 teor2345 changed the title Automatically reject connections from outdated peers after network upgrade activation Automatically reject new connections from outdated peers after network upgrade activation Jun 8, 2021
@teor2345 teor2345 self-assigned this Jun 14, 2021
@teor2345 teor2345 changed the title Automatically reject new connections from outdated peers after network upgrade activation After network upgrade activation, reject new connections from outdated peers Jun 14, 2021
@mpguerra mpguerra linked a pull request Jun 29, 2021 that will close this issue
3 tasks
@jvff jvff self-assigned this Jul 12, 2021
@teor2345
Copy link
Contributor Author

teor2345 commented Aug 9, 2021

Closed by #2519.

@teor2345 teor2345 closed this as completed Aug 9, 2021
@teor2345 teor2345 linked a pull request Aug 16, 2021 that will close this issue
3 tasks
@teor2345 teor2345 mentioned this issue Aug 17, 2021
59 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-design Category: Software design work C-security Category: Security issues I-heavy Problems with excessive memory, disk, or CPU usage I-slow Problems with performance or responsiveness I-unbounded-growth Zebra keeps using resources, without any limit NU-5 Network Upgrade: NU5 specific tasks
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants