feat: Allow multiple entries in graffiti file via YAML configuration
PR Description
Adds support for multiple graffiti entries in a YAML configuration file, similar to Prysm's functionality. This implementation allows validators to use different graffiti messages based on various criteria:
- Specific graffiti for individual validators by public key
- Ordered list of graffiti messages used in sequence
- Random selection from a list of graffiti messages
- Default fallback graffiti
Maintains backward compatibility with the existing single-value text format.
Fixed Issue(s)
fixes #3471
Documentation
- [x] I thought about documentation and added the
doc-change-requiredlabel to this PR if updates are required.
Changelog
- [x] I thought about adding a changelog entry, and added one if I deemed necessary.
Thanks for raising this @CreeptoGengar , I'm just starting to have a look at the details here.
To fix spotless task, you can run gradle spotlessJavaApply in the teku folder and it'll update some files to conform to our coding standards and you'll be able to commit those changes...
@rolfyone do i need to make ci/circleci: acceptanceTests green? also corrected what you asked me for
@lucassaldanha tnx for pointing this out, deleted the docs directory
@rolfyone do i need to make ci/circleci: acceptanceTests green? also corrected what you asked me for ta - will have a look. The AT were green when they ran last, if they break i can see if they're related to your change etc...
Reviewing this code, I have identified something we could do to improve our handling of errors when getting the graffiti from any GraffitiProvider implementation.
In Validator.java we do the following:
public Optional<Bytes32> getGraffiti() {
return graffitiProvider.get();
}
This means that each GraffitiProvider implementation must properly handle any errors, risking it bubbling up all the way and eventually impacting block production. To be on the safer side, I recon we should change it so we capture ANY errors from GraffitiProvider#get and, upon an exception, we return empty. Something like this:
public Optional<Bytes32> getGraffiti() {
try {
return graffitiProvider.get();
} catch(final Exception e) {
LOG.warn("Error getting graffiti from graffiti provider: {}", e);
return Optional.empty();
}
}
This way we know that regardless of what happens on GraffitiProvider, we won't impact our block production. Of course we would like to have unit tests to verify this behaviour.
All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.
I have read the CLA Document and I hereby sign the CLA
Closing PR due to inactivity.