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

Log graffiti on validator initialization #6662

Open
wants to merge 3 commits into
base: unstable
Choose a base branch
from

Conversation

shayanb
Copy link

@shayanb shayanb commented Dec 5, 2024

Issue Addressed

No related issues found, but this feature was discussed shortly with @michaelsproul here: #6635 (comment)

Proposed Changes

Print out the initial graffiti set to the logs when initializing the validators.

Additional Info

This can help to see if the initial graffiti is set correctly.

I wasn't able to find an easy way to test this, however the test suite are all passing locally.

@chong-he chong-he added ready-for-review The code is ready for review UX-and-logs labels Dec 11, 2024
@@ -1285,6 +1284,7 @@ impl InitializedValidators {
"Enabled validator";
"signing_method" => "local_keystore",
"voting_pubkey" => format!("{:?}", def.voting_public_key),
"graffiti" => format!("{:?}", def.graffiti)
Copy link
Member

Choose a reason for hiding this comment

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

Just noting that this will only log the graffiti for a validator if it is specified in the validator_definitions.yml, not other sources (e.g. graffiti file).

We're also printing the Debug representation here which will be a bit ugly like None or Some("graffiti"). Instead it would be good to format the None case explicitly and just display the string if it is Some. Something like def.graffiti.map_or("none".to_string(), |graffiti| graffiti.to_string()). You may need to implement Display for GraffitiString.

@michaelsproul michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UX-and-logs waiting-on-author The reviewer has suggested changes and awaits thier implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants