Include EL client info in graffiti

[ensi321]

Problem description

Upon agreement reached in ethereum/execution-apis#517 to expose EL client info (ie. standardized client code and version) via Engine API engine_getClientVersionV1, it is sensible to include such info in the graffiti such that EL client diversity could be accurately measured.

Solution description

Currently the default graffiti is ${lodestarPackageName}-${version} with the option to override it with user-defined value passed by --graffiti flag.

I suggest the user-defined value to be appended after the necessary client info (EL info from engine_getClientVersionV1 and CL info from itself) instead of erasing it altogether.

The discussion in the Engine API PR suggested to use the format laid out here but it is not officially decided yet.

Additional context

No response

[nflaig]

I suggest the user-defined value to be appended after the necessary client info (EL info from engine_getClientVersionV1 and CL info from itself) instead of erasing it altogether.

We must give users the option to customize their graffiti or even set an empty graffiti. I'd rather see this as an extension to the default graffiti.

And if users want client version info to be included in addition to their custom graffiti, we could make it opt-in by adding another CLI flag.

[philknows]

I think we might be the last client team right now that needs to implement this 😅

[philknows]

We also have a --private flag which hides the agent version of our client which is an opt-out option for privacy. Perhaps this is something to also include as the spec change leading to this is meant to be opt-out.

[ensi321]

We must give users the option to customize their graffiti or even set an empty graffiti. I'd rather see this as an extension to the default graffiti.

And if users want client version info to be included in addition to their custom graffiti, we could make it opt-in by adding another CLI flag.

@nflaig I like Teku's approach: Include user's graffiti, if there is enough space left in the graffiti, we will try to squeeze in the client info at the end:

• If there is 13 bytes left: <space>ELcode|2bytecommit|CLcode|2bytecommit
• If there is 5 bytes left: <space>ELcode|CLcode
• If < 5 bytes left: don't include client info

This behaviour is enabled by default, and can be disabled via a flag.

Teku has a CLIENT_CODES option in their flag for users who prefer to expose client name but hide the commit versions for privacy reason.

I wonder if we need to give users flexibility on this, or just a on/off flag be enough?

[nflaig]

I think we might be the last client team right now that needs to implement this 😅

Good news, at least Prysm does seem to have it yet as well https://github.com/prysmaticlabs/prysm/issues/13558, and not sure about Nimbus

we will try to squeeze in the client info at the end

How would this look technically? As far as I understand this information is only available to the beacon node by calling the execution client? Imo the graffiti needs to be dictated by the validator client as this is where it is configured

commit versions for privacy reason

This would rather be a security concern but we already leak too much information on p2p so not sure how much it matters at this point, but even without including the commit hash, if we append client details to a user defined graffiti, it is trivial to correlate validators by simply analyzing block proposals.

This behaviour is enabled by default, and can be disabled via a flag.

I am against enabling this by default as it is not something I would like as a node operator myself.

[philknows]

This would rather be a security concern but we already leak too much information on p2p so not sure how much it matters at this point, but even without including the commit hash, if we append client details to a user defined graffiti, it is trivial to correlate validators by simply analyzing block proposals.

We already provide AgentVersion by default on p2p don't we? And this also includes the version commit IIRC. And of course, we already append version + commit to the graffiti already. All of these are opt-out. So by keeping with standards, this should function the same way. I get the privacy aspect, but based on the history we have of already exposing version and commit numbers, we haven't seen any instances of targeted attacks using this data, but the insight has been and will be useful as described in https://github.com/ethereum/execution-apis/pull/517. There will always be a way to opt-out.

[nflaig]

We already provide AgentVersion by default on p2p don't we?

On p2p it's just the client name, no version / commit details https://github.com/ChainSafe/lodestar/blob/8e875c61315fd5cf36bcf4cbad7dc66969d55756/packages/cli/src/cmds/beacon/handler.ts#L200

And of course, we already append version + commit to the graffiti already

This is a good default we can extend with EL client info, however, a user explicitly specifying --graffiti already opted out of this. If we append client details by default to the user specified graffiti, the user has to opt out once more, but this time it is a flag which has to be set on the beacon node side.

[philknows]

I see the argument here. I think the point of this execution API change was to capture the data from those who don't bother with opting out of the default because they presumably don't care enough to do so.

So if that is the preference, we should just modify our default graffiti to this new format and won't have to figure out how to deal with mixing it into custom graffiti set by the flag? Open to discussion.