Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

sync: remove broken eth_protocol_version method #11473

Merged
merged 2 commits into from
Feb 10, 2020

Conversation

ordian
Copy link
Collaborator

@ordian ordian commented Feb 10, 2020

It seems that this method is currently unused, but could be used in #11472.
ATM, it returns the same version as
if warp_protocol { io.protocol_version(&WARP_SYNC_PROTOCOL_ID, peer) } else { ETH_PROTOCOL_VERSION_63.0 };

@ordian ordian added A0-pleasereview 🤓 Pull request needs code review. A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). labels Feb 10, 2020
@ordian
Copy link
Collaborator Author

ordian commented Feb 10, 2020

Alternatively, we can delete this method altogether (suggested by @grbIzl), any thoughts @dvdplm @niklasad1 ?

@vorot93
Copy link

vorot93 commented Feb 10, 2020

I think it is superfluous and would be in favor of removing it altogether.

@ordian ordian changed the title sync: fix eth_protocol_version sync: remove broken eth_protocol_version method Feb 10, 2020
Copy link
Collaborator

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

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

looks good to me

@ordian ordian added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 10, 2020
Copy link
Collaborator

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

lgtm

@dvdplm dvdplm merged commit c5123e1 into master Feb 10, 2020
@dvdplm dvdplm deleted the ao-fix-eth_protocol_version branch February 10, 2020 20:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants