teku icon indicating copy to clipboard operation
teku copied to clipboard

feat: Allow multiple entries in graffiti file via YAML configuration

Open CreeptoGengar opened this issue 8 months ago • 3 comments

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-required label to this PR if updates are required.

Changelog

  • [x] I thought about adding a changelog entry, and added one if I deemed necessary.

CreeptoGengar avatar May 06 '25 13:05 CreeptoGengar

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 avatar May 07 '25 04:05 rolfyone

@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

CreeptoGengar avatar May 07 '25 17:05 CreeptoGengar

@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...

rolfyone avatar May 14 '25 04:05 rolfyone

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.

lucassaldanha avatar Jun 27 '25 02:06 lucassaldanha

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

github-actions[bot] avatar Aug 07 '25 21:08 github-actions[bot]

I have read the CLA Document and I hereby sign the CLA

CreeptoGengar avatar Aug 07 '25 21:08 CreeptoGengar

Closing PR due to inactivity.

lucassaldanha avatar Sep 30 '25 02:09 lucassaldanha