Graffiti proposal design doc #15983
Graffiti proposal design doc #15983
Conversation
| GE 168d PR 63af = "GE168dPR63af" | ||
| ``` | ||
|
|
||
| - **EL_CODE**: 2-letter code (GE=Geth, NM=Nethermind, BU=Besu, RH=Reth, EG=Erigon) |
There was a problem hiding this comment.
We can link to the execution spec where the abbreviations are defined: src/engine/identification.md.
|
|
||
| **Prysm hasn't yet implemented the ecosystem standard.** Prysm currently leaves graffiti empty (when no user graffiti is configured), providing no on-chain visibility into client versions. | ||
|
|
||
| **Why it matters**: On-chain visibility of client diversity is much easier with standardized graffiti. |
There was a problem hiding this comment.
nitpick: "Why it matters" is a common AI verbal tick that I would not like to see propagating through our design docs 🙏
| | No graffiti | `GE168dPR63af` ✨ **NEW** | | ||
| | `--graffiti "🚀"` | `🚀` (unchanged) | | ||
| | Proposer settings | Custom graffiti (unchanged) | | ||
| | Old Geth (no API) | `Prysm/v6.1.0` (graceful fallback) | |
There was a problem hiding this comment.
Can we follow the "flexible standard" that was proposed alongside the engine API PR? These examples imply that we would only set the custom graffiti if no user graffiti is specified.
If we do that it could also be nice to emit a log to warn the user that they are using up valuable graffiti real estate that could be used for client diversity. For example look at this block: Twinstake for EtherFi. Maybe they would be happy to just set the flag to EtherFi if they knew it helped client diversity analysis. As an aside, it's a little pointless for the big stakers to set custom graffiti since the chain explorers know who they are anyway.
| 2. **Beacon Node RPC** receives the request with VC's graffiti (empty if not set) | ||
| 3. **Graffiti Service** decides which graffiti to use based on priority | ||
| 4. **Cache** provides pre-fetched EL version (if needed for auto-generation) | ||
| 5. **Block returned** to VC with resolved graffiti |
There was a problem hiding this comment.
nitpick: I think we should try to make design docs lean where we can. I feel like a block diagram and in particular including the validators role here is overkill or maybe just redundant with general knowledge of how block proposal works?
| | **Engine Version Cache** | Thread-safe cache storing EL version with 6-epoch TTL to avoid RPC latency | | ||
| | **Background Refresh** | Goroutine that pre-warms cache every 2 epochs so block production never waits | | ||
| | **Engine Client Extension** | Adds `GetClientVersion()` RPC method calling `engine_getClientVersionV1` | | ||
| | **Version Helpers** | Utility functions to extract/normalize commit hashes without runtime git calls | |
There was a problem hiding this comment.
Why would there be runtime git calls? As the doc states above, this information is baked into the binary via version.GetCommitPrefix().
| | Component | One-Line Summary | | ||
| |-----------|-----------------| | ||
| | **GraffitiResolver Interface** | Interface that defines `ResolveGraffiti(vcGraffiti)` method for decoupling RPC layer from implementation | | ||
| | **Graffiti Service** | Resolves graffiti based on priority: VC graffiti > auto-generated > default | |
There was a problem hiding this comment.
I don't think this needs to be it's own service. It could be a runloop inside beacon-chain/execution/service.go.
| ## Open Questions | ||
|
|
||
| 1. **Cache timing**: Uses mainnet values (6 epochs, 2 epochs) for all networks. Add env var overrides for testing? | ||
| 2. **Missing EL commit**: Use "0000" placeholder or fall back to default? |
There was a problem hiding this comment.
IDK what "fall back to default" would mean? zero value makes sense.
|
|
||
| ## Open Questions | ||
|
|
||
| 1. **Cache timing**: Uses mainnet values (6 epochs, 2 epochs) for all networks. Add env var overrides for testing? |
There was a problem hiding this comment.
mainnet values (6 epochs, 2 epochs) doesn't track for me, what does that mean?
This API call should be incredibly cheap for the EL to serve. I think you can do it once at startup, and then a couple times an epoch to be on the safe side. Idea to avoid any epoch boundaries: slot % 8 == 4 means you'd do it 4 times an epoch, never too close to the beginning or end of an epoch.
| |-----------|-----------------| | ||
| | **GraffitiResolver Interface** | Interface that defines `ResolveGraffiti(vcGraffiti)` method for decoupling RPC layer from implementation | | ||
| | **Graffiti Service** | Resolves graffiti based on priority: VC graffiti > auto-generated > default | | ||
| | **Engine Version Cache** | Thread-safe cache storing EL version with 6-epoch TTL to avoid RPC latency | |
There was a problem hiding this comment.
You do not need a TTL. Just update the value whenever you can and use the last good value. If the call has never succeeded then the EL abbreviation will be blank. When you generate the client string you can check if the EL abbreviation is set, and if it's not, generate the graffiti with the CL abbrev + commit only.
| ### Priority Order | ||
| ```go | ||
| func (g *GraffitiInfo) GenerateGraffiti() [32]byte { | ||
| if userGraffiti != "" { |
There was a problem hiding this comment.
What do you think about implementing Mark's "flexible standard" so that we can use whatever space is leftover after the user's message to pack in the impl information?
What type of PR is this?
What does this PR do? Why is it needed?
Which issues(s) does this PR fix?
Fixes #
Other notes for review
Acknowledgements