-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Prysmctl Command to Request Beacon Nodes for Block Ranges Over P2P #11035
Conversation
CurrentFork() *ethpb.Fork | ||
GenesisFetcher | ||
TimeFetcher | ||
} |
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.
What's up with these changes?
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.
See some of the self-review comments above. I don't have a great name for this composed interface, but it's the minimum amount of methods needed by the p2p service regarding the chain itself. During p2p and sync, we need the current fork, the genesis time, genesis validators root, and current slot. This new iface encompasses all that
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.
Reducing interface surface area by composing stuff into something called ForkFetcher. This is the minimum set of methods needed by p2p and sync from the blockchain interface
@@ -332,6 +332,31 @@ func (s *Service) isPeerAtLimit(inbound bool) bool { | |||
return activePeers >= maxPeers || numOfConns >= maxPeers | |||
} | |||
|
|||
// PeersFromStringAddrs convers peer raw ENRs into multiaddrs for p2p. | |||
func PeersFromStringAddrs(addrs []string) ([]ma.Multiaddr, error) { |
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 is the wrong direction imo, we should be having all this under the network
package as a sub-package for all these helpers.
@@ -15,18 +15,30 @@ import ( | |||
"github.com/prysmaticlabs/prysm/runtime/version" | |||
) | |||
|
|||
// MultiAddressBuilder takes in an ip address string and port to produce a go multiaddr format. | |||
func MultiAddressBuilder(ipAddr string, port uint) (ma.Multiaddr, error) { |
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.
Same thoughts here
What type of PR is this?
What does this PR do? Why is it needed?
We lack tooling to spin up a local libp2p host that can peer with beacon nodes and issue proper requests for data. This is important for load testing new features that depend on the p2p stack, such as #11010. This PR adds in a subcommand to
prysmctl
which allows for querying a specific beacon node peer for a range of beacon blocks over p2p. Here's how to use it:This connects to a beacon node via gRPC as well to retrieve the necessary data to handshake with beacon nodes before making proper requests for data. The tool then outputs data to stdout showing the request timing and checking if the request worked or not.
To support custom configs, remember to specify the
--chain-config-file
to the command as well. This is crucial to get the feature to work well with the Sepolia chain, for example.Example on Sepolia:
Which issues(s) does this PR fix?
Part of #10589