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.

Sign up for GitHub

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

Jump to bottom

Remove pre eth66 network items #4749

Merged
merged 3 commits into from
Oct 13, 2022
Merged

Remove pre eth66 network items #4749

merged 3 commits into from
Oct 13, 2022

Conversation

marcindsobczak
Copy link
Contributor

Fixes | Closes | Resolves #3491

Changes:

  • drop old eth capabilities (eth62, eth63, eth64, eth65)
  • remove handlers
  • adjust tests

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Other (please describe):

Testing

Requires testing

  • Yes
  • No

In case you checked yes, did you write tests??

  • Yes (adjusted existing ones)
  • No

@marcindsobczak marcindsobczak marked this pull request as ready for review October 12, 2022 12:38
asdacap
asdacap reviewed Oct 13, 2022
View reviewed changes
src/Nethermind/Nethermind.Network/ProtocolsManager.cs
@@ -197,10 +194,6 @@ public void AddProtocol(string code, Func<ISession, IProtocolHandler> factory)
{
var ethHandler = version switch
{
62 => new Eth62ProtocolHandler(session, _serializer, _stats, _syncServer, _txPool, _gossipPolicy, _logManager),
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we removing the files also?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, we need these files. Eth66ProtocolHandler is extending Eth65ProtocolHandler, which is extending Eth64ProtocolHandler etc. We are not creating instances of old protocols, but files are still needed to use methods from them

@marcindsobczak marcindsobczak merged commit 6198f06 into master Oct 13, 2022
@marcindsobczak marcindsobczak deleted the feature/drop_old_eth_capabilities branch October 13, 2022 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asdacap asdacap asdacap left review comments

@LukaszRozmej LukaszRozmej LukaszRozmej approved these changes

Assignees
No one assigned
Labels
network
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove pre eth66 network items.
3 participants
@marcindsobczak @asdacap @LukaszRozmej