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

prysmctl: output proposer settings #12181

Merged
merged 55 commits into from
Jun 6, 2023

Conversation

james-prysm
Copy link
Contributor

@james-prysm james-prysm commented Mar 23, 2023

What type of PR is this?

Feature

What does this PR do? Why is it needed?

introduces a new command under prysmctl to query the validator client against the keymanagerAPIs and get the proposer settings ( namely the fee recipient) registered on the validator client. optionally you can output a proposer settings file which would be useful for webui migrations.

prysm currently requires --web flag against the validator client to enable its apis, in this process of startup an oauth token is created which is used in the --token flag below.

example command line:

prysmctl validator proposer-settings --validator-client-host=http://127.0.0.1:7500 --token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.e30.VXjrSItV_Kmwg_XilpscyPm2SPIsstytYLtr_AuJI8I --settings-path=./settings.json

example of a successful run:

time="2023-03-23T20:51:07-05:00" level=info msg="===============DISPLAYING CURRENT PROPOSER SETTINGS==============="
time="2023-03-23T20:51:07-05:00" level=info msg="validator: 0x855ae9c6184d6edd46351b375f16f541b2d33b0ed0da9be4571b13938588aee840ba606a946f0e8023ae3a4b2a43b4d4. fee-recipient: 0xb698D697092822185bF0311052215d5B5e1F3944"
time="2023-03-23T20:51:07-05:00" level=info msg="validator: 0x844ae9c6184d6edd46351b375f16f541b2d33b0ed0da9be4571b13938588aee840ba606a946f0e8023ae3a4b2a43b4d4. fee-recipient: 0xb698D697092822185bF0311052215d5B5e1F3944"
time="2023-03-23T20:51:07-05:00" level=info msg="the default fee recipient is set to 0xb698D697092822185bF0311052215d5B5e1F3944"
time="2023-03-23T20:51:07-05:00" level=info msg="successfully created `./testdata/settings.json`. settings can be imported into validator client using --proposer-settings-file flag. does not include custom builder settings."

@james-prysm james-prysm marked this pull request as ready for review March 24, 2023 02:08
@james-prysm james-prysm requested a review from a team as a code owner March 24, 2023 02:08
@james-prysm james-prysm requested a review from potuz March 27, 2023 16:17
@james-prysm james-prysm dismissed a stale review via df44ccb April 17, 2023 22:10
}

// Client provides a collection of helper methods for calling the Eth Beacon Node API endpoints.
type Client struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is already namespaced by its package. Renaming it to BeaconClient is not necessary and causes the package + type name to stutter.

api/client/options.go Outdated Show resolved Hide resolved
api/client/options.go Outdated Show resolved Hide resolved
api/client/options.go Outdated Show resolved Hide resolved
Copy link
Contributor

@rkapka rkapka left a comment

Choose a reason for hiding this comment

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

validateIsExecutionAddress has a bug

"github.com/pkg/errors"
)

// Client provides a collection of helper methods for calling the Eth Beacon Node API endpoints.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is more generic and can be used outside of Beacon API. I would describe this type as an HTTP wrapper.

api/client/errors.go Outdated Show resolved Hide resolved
api/client/errors.go Outdated Show resolved Hide resolved
"github.com/pkg/errors"
)

// ErrMalformedHostname is used to indicate if a host name's format is incorrect while providing the port.
Copy link
Contributor

Choose a reason for hiding this comment

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

This sentence is unclear to me. Wouldn't ErrMalformedHostname is used to indicate if a host name's format is incorrect be enough?

api/client/options.go Outdated Show resolved Hide resolved
cmd/prysmctl/validator/proposer_settings.go Outdated Show resolved Hide resolved
cmd/prysmctl/validator/proposer_settings.go Outdated Show resolved Hide resolved
cmd/prysmctl/validator/proposer_settings.go Outdated Show resolved Hide resolved
cmd/prysmctl/validator/proposer_settings.go Outdated Show resolved Hide resolved
return nil
}

func validateIsExecutionAddress(input string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

This function is incorrect because 0xpppp... with the correct length will pass. You should use bytesutil.IsHex + the length check. It would be nice to add some unit tests too.

james-prysm and others added 17 commits June 5, 2023 13:01
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
import "fmt"

// ErrNoFlag takes a flag name and returns a formatted error representing no flag was provided.
func ErrNoFlag(flagName string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it can be unexported

return nil
}

func ValidateIsExecutionAddress(input string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it can be unexported.

@prylabs-bulldozer prylabs-bulldozer bot merged commit 6672d14 into develop Jun 6, 2023
15 of 16 checks passed
@prylabs-bulldozer prylabs-bulldozer bot deleted the prysmctl-proposer-settings branch June 6, 2023 17:03
james-prysm added a commit that referenced this pull request Jun 7, 2023
* wip proposer settings

* WIP validator client APIs

* adding proposer settings output

* adding unit tests

* fixing linting

* fixing deepsource issues

* fixing e2e

* fixing deep source issue

* updating naming to not stutter

* updating bazel

* fixing linting error

* reverting comment

* adding builder settings

* gaz

* Update validator/client/validator.go

Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com>

* adding comments

* adding some tests

* gaz

* Update cmd/prysmctl/validator/proposer_settings.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update cmd/prysmctl/validator/proposer_settings.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update cmd/prysmctl/validator/proposer_settings.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update cmd/prysmctl/validator/proposer_settings.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update api/client/options.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update api/client/options.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update api/client/errors.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update api/client/options.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update api/client/options.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update api/client/validator/client.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update cmd/prysmctl/validator/cmd.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update api/client/validator/client.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update api/client/validator/client.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update cmd/prysmctl/validator/proposer_settings.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update api/client/errors.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* fixing feedback

* fixing unit test

* addressign comments

---------

Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com>
Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants