-
-
Notifications
You must be signed in to change notification settings - Fork 306
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
Modular Reqresp Package #4775
Modular Reqresp Package #4775
Conversation
packages/reqresp/.babel-register
Outdated
@@ -0,0 +1,15 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file can be deleted
packages/reqresp/src/types.ts
Outdated
LightClientBootstrap = "light_client_bootstrap", | ||
LightClientUpdatesByRange = "light_client_updates_by_range", | ||
LightClientFinalityUpdate = "light_client_finality_update", | ||
LightClientOptimisticUpdate = "light_client_optimistic_update", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reqResp package is agnostic of full node specifics. All of this should be in the beacon-node package only
8cca03b
to
38b9225
Compare
Performance Report✔️ no performance regression detected Full benchmark results
|
@@ -1,10 +0,0 @@ | |||
import {expect} from "chai"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the unit tests are deleted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That we discussed to add and fix the unit tests in a separate PR to not block this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! One minor comment for latter on test fixing
/** | ||
* https://github.com/ethereum/consensus-specs/blob/v1.2.0/specs/phase0/p2p-interface.md#protocol-identification | ||
*/ | ||
export function parseProtocolID(protocolId: string): Protocol { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function was not in source because it's only used in tests. I try to not keep any code in source that's only for testing
Motivation
Make the
reqresp
modular so it can be consumed among light-client and beacon-node easily.Description
Extract the reqresp out of beacon node and create its own package. Made the messages and handlers generic.
Steps to test or reproduce
Run unit tests.