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

Load graffiti from file #8041

Merged
merged 17 commits into from
Dec 4, 2020
Merged

Load graffiti from file #8041

merged 17 commits into from
Dec 4, 2020

Conversation

terencechain
Copy link
Member

@terencechain terencechain commented Dec 3, 2020

Accompany design doc: https://hackmd.io/9su3BOB8TvOMITOsqHtO_A?view

Upon approval I will add a section for this in: https://docs.prylabs.network/docs/prysm-usage

Tested all the possible scenarios in run time with interop mode

@terencechain terencechain self-assigned this Dec 4, 2020
@terencechain terencechain added the Ready For Review A pull request ready for code review label Dec 4, 2020
@terencechain terencechain marked this pull request as ready for review December 4, 2020 18:33
@terencechain terencechain requested a review from a team as a code owner December 4, 2020 18:33
Comment on lines 303 to 306
idx, err := v.validatorClient.ValidatorIndex(ctx, &ethpb.ValidatorIndexRequest{PublicKey: pubKey[:]})
if err != nil {
return []byte{}, err
}
Copy link
Member

Choose a reason for hiding this comment

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

Can this be cached some how? We know this information from the duties as well

Copy link
Member Author

Choose a reason for hiding this comment

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

Discussed offline, there's no cache but this is ok. The call is cheap and proposing block is not a frequent event

Comment on lines 380 to 382
if err != nil {
return err
}
Copy link
Member

Choose a reason for hiding this comment

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

Maybe just log a warning here? It would be sad if parsing a graffiti file failed and the validator remained offline

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point!

type Graffiti struct {
Default string `yaml:"default,omitempty"`
Random []string `yaml:"random,omitempty"`
Specific map[uint64]string `yaml:"specific,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

In your design, you call this "validators". Why is it "specific" here? I

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed it to specific in the design doc but i forgot to update the example sections.

You mentioned specific in our chat and I liked it better than validators

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good. No preference here, just wanting consistency. thanks!

Comment on lines +89 to +94
b := []byte(`default: "Rice"
random:
- "Sushi"
- "Ramen"
- "Takoyaki"
`)
Copy link
Member

Choose a reason for hiding this comment

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

yum

@prestonvanloon prestonvanloon added this to the v1.0.4 milestone Dec 4, 2020
@prylabs-bulldozer prylabs-bulldozer bot merged commit b4437e6 into develop Dec 4, 2020
@delete-merged-branch delete-merged-branch bot deleted the graffiti branch December 4, 2020 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Load graffiti from file and select different graffiti per validator
2 participants