Repository navigation
gloas: MEV-Boost/commit-boost docs, sidecar AuthData warning, masked builder URLs, not_published outcome - #3152
Conversation
…r entries without AuthData After Gloas the beacon node contacts each configured builder itself, so a MEV-Boost or commit-boost sidecar is optional. An operator who keeps one needs an entry per relay with URL set to the sidecar and AuthData set to the relay's identity; omitting AuthData signs the sidecar's own hostname, which matches no relay, so the entry silently gets no bids. - Warn at startup for each builder entry that omits AuthData while its URL host looks local (loopback/private/link-local IPs, single-label names, .localhost/.local/.internal), logging the hostname only. - Document running a sidecar after Gloas, that URLs may differ per operator, and scope the pre-Gloas sections to before the fork.
Codecov Report✅ All modified and coverable lines are covered by tests. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
[Low impact] The PR appears safe to merge; no actionable issues were found. SummaryAdds a startup warning for local-looking builder entries without explicit
Reviews (1) · Last reviewed commit: "gloas: document MEV-Boost/commit-boost a..." · Reviewed by Greptile |
…refresh pre-Gloas docs - Builder URLs may carry credentials (a basic-auth password, an API key in the path). The request-auth round logged them verbatim; it now logs them masked. The masking moves from goclient.RedactBeaconAddr into format.RedactURL, shared by both, and resolved builder entries carry a RedactedURL computed once at load. - The BuilderConfig comment now says the URL is the one field that may differ per operator, matching the docs. - The pre-Gloas docs described the v1 blinded-block endpoints and a Prysm limitation of them. SSV uses produceBlockV3 now, which Prysm serves, so the Prysm note goes and the endpoints are corrected; the "builder proposals enabled" toggle they mention no longer exists.
…ork now
The "Edge cases outcomes" scenarios came from 2023 testing, before
consensus switched to always running on blinded blocks. Scenario 3 had
operators without MEV rejecting a builder block ("blinded blocks are not
supported") and failing the round; that check no longer exists, and the
value check doesn't depend on the builder setup. Scenarios 1 and 2
mostly recorded relay behavior rather than SSV's.
Replace them with a short section on clusters with mixed builder setups,
and correct the blinded-proposal paragraph: consensus always runs on the
blinded form, the deciding round's leader submits its own full block,
and every other operator submits the blinded block.
… explain non-leader submits The URL exception to the identical-builder-config rule left out a trap: AuthData defaults to the URL's hostname, so operators whose hostnames differ (each pointing at its own sidecar) must set AuthData explicitly, or the signed bytes diverge and the builder is silently disabled. Say so in the BuilderConfig comment and the docs, and carry the qualified exception into the Builders env-description and the example config, which still required entries to be configured identically. The Lighthouse builder link no longer resolves and the Prysm one redirects to the generic parameters page; point both at their MEV builder pages. Replace the pre-Gloas TODO about non-leaders submitting the decided block with why they do: ssv-spec has every operator submit, and it adds redundancy for builder blocks. Their failures, when their beacon node can't unblind the block, are expected.
… not_published Before Gloas every operator submits the decided block, but a beacon node can't unblind a block another operator's beacon node produced. Those expected misses logged an error, counted a failed submission and concluded the proposal duty failed, even when the block landed, which made proposal-failure alerts noisy. The runner now remembers the blinded SSZ of every block it fetches, not only full ones, and compares the decided value with it. A failed submit of another operator's block logs at debug, skips the failed-submission counter and concludes the duty with a new not_published outcome. A failed submit of the operator's own block, full or blinded, still fails the duty. Also list the builder docs of the other beacon clients (Grandine, Lodestar, Nimbus, Teku) and describe the new outcome in the doc.
- Log a failed submit of another operator's pre-Gloas block at Info, so the duty's Info log doesn't stop silently after "submitting block proposal"; the not_published outcome itself stays at Debug. - Rename BuilderEntry.DefaultAuthDataOnLocalHost to LocalHostWithoutAuthData: it returns the bare host, not the default AuthData. The warning's index field becomes builder_entry_index, and its test matches the warning by message. - Drop frozenBuilderRef: frozen request auths point at the resolved builder entries instead of copying four of their fields. - Log the leader's cache-mismatch as two block roots instead of two hex-encoded blocks, and only when Debug is enabled, since hashing the full block would otherwise delay the submit. - Docs: say the beacon node contacts the builders listed on the produceBlockV4 POST, and note that MEV-Boost's Gloas routing is still in review (flashbots/mev-boost#914). - Shorten the comments this PR added or touched, and the example config's Builders note.
y0sher
left a comment
There was a problem hiding this comment.
LGTM. One heads-up for after this lands: ssv.runner.duty.outcome gains a new not_published value, and pre-Gloas submit misses on another operator's block move out of failed. Any dashboard panel or alert that counts failed proposer duties will drop when this ships, so it's worth adding not_published wherever the total outcome breakdown matters.
Why
After Gloas, the beacon node contacts each configured builder itself over the
produceBlockV4POST (beacon-APIs#630), so SSV doesn't need MEV-Boost or commit-boost. Operators may still run one: commit-boost has merged its Gloas builder-API endpoints (Commit-Boost/commit-boost-client#505), and MEV-Boost's are in review (flashbots/mev-boost#914). Both forward each bid request to the one relay named by the auth data it carries, so each needs one SSV builder entry per relay, withURLset to the sidecar andAuthDataset to the relay's identity.docs/EXTERNAL_BUILDERS.mddidn't say any of this, and its pre-Gloas sections didn't say they stop applying at the fork. The config also has a silent trap: an entry that points at a sidecar withoutAuthDatadefaults to the sidecar's own hostname (localhost,mev-boost). That matches no relay, so the sidecar rejects every request and the entry gets no bids, with nothing in the logs.Four smaller issues came up along the way:
BuilderConfigcomment, theBuildersenv-description and the example config all said the whole config must be identical across operators. The URL isn't signed, though, and with a sidecar it naturally differs per operator. That only works ifAuthDatais set explicitly, because the default comes from the URL's hostname.produceBlockV3(/eth/v3/validator/blocks), which Prysm has served since its Deneb work in 2023. The "Edge cases outcomes" scenarios came from 2023 testing, before consensus switched to always running on blinded blocks (feat: blind all blocks (by iurii) #2569). In Scenario 3, operators without MEV rejected a builder block with "blinded blocks are not supported" and the round failed. That error no longer exists, and the value check doesn't depend on the builder setup.failed, even when the block landed.What changed
BuilderEntry.LocalHostWithoutAuthDatareports when an entry omitsAuthDataand its URL host looks local: a loopback, private (IPv6 unique-local included), link-local or unspecified IP; a single-label name (localhost, a Docker service name); or a.localhost,.localor.internalname.resolveAndValidatewarns at startup for each such entry, logging the entry index (builder_entry_index) and the hostname only. This is a warning, not an error, because a builder genuinely reached over a private network trips the same check.goclient.RedactBeaconAddrdid (password, path and query values, as go-eth2-client masksClient.Address()) moves intoformat.RedactURL, so the beacon and builder paths share one implementation.RedactBeaconAddrbehaves exactly as before; its existing tests are unchanged.ResolvedBuilderEntrygainsRedactedURL, computed once inResolveBuilderConfig.frozenBuilderRef.AuthDataexplicitly. TheBuilderConfigcomment, theBuildersenv-description (shown byssvnode doc),config/config.example.yamland the doc all say so.TODOasking whether non-leaders should submit the decided block at all is replaced with a comment explaining why they do: ssv-spec'ssubmitDecidedBlockhas every operator submit, and it adds redundancy for builder blocks.not_publishedoutcome inssv.runner.duty.outcome.docs/EXTERNAL_BUILDERS.md:AuthData, a YAML example and a one-liner for the hex. It also says clusters can mix direct and sidecar setups, since onlyAuthDataand the slot are signed. Because MEV-Boost's Gloas endpoints are still in review (Gloas flashbots/mev-boost#914), it notes that MEV-Boost's routing may still change.URLisn't signed and may differ per operator.produceBlockV3,/eth/v2/beacon/blinded_blocks,/eth/v2/beacon/blocks) and describes the current flow. Consensus always runs on the blinded form. The deciding round's leader submits its own full block, and every other operator submits the blinded block. The "if builder proposals are enabled" qualifiers are gone, because registrations are always scheduled and the beacon node decides blinded vs. full. The obsolete Prysm "Known issues" entry is removed.builders.htmlno longer exists, and the Prysm link only redirected to the generic parameters page. Both now point at the clients' MEV builder pages, and the list gains Grandine, Lodestar, Nimbus and Teku.not_published.Tests
TestBuilderEntry_LocalHostWithoutAuthDatacovers each class of local host, including an IPv4-mapped IPv6 loopback and a name with a trailing dot. It also checks that public hosts, an address just outside 172.16.0.0/12, an entry with explicitAuthDataand an unparsable URL don't match.Test_resolveAndValidate_builderLocalHostWithoutAuthDatachecks that only the local entry withoutAuthDatawarns, and that the log carries the hostname rather than the URL's credentials.TestRedactURLcovers the masking cases, including a username alone (a relay pubkey in MEV-Boost-style URLs), which is kept.TestProposerPreferencesRunner_requestAuthLogsRedactedURLdrives a request auth to quorum for a builder URL carrying a password and a path API key. It checks that no log line contains either one, and that the builder-preferences submit still receives the URL verbatim. I checked that it fails when the raw URL is logged again.TestProposerRunnerProcessPostConsensusSubmitFailureOfAnotherOperatorsBlockIsNotPublishedchecks that a failed submit of another operator's block concludesnot_publishedwithout returning an error. I checked that it fails without the classification.TestProposerRunnerProcessPostConsensusSubmitFailureOfOwnBlockFailsDutychecks that the operator's own block, full or blinded, still concludesfailed.TestProposerRunnerProcessPreConsensusCachesOnlyBlindedBlockSSZreplaces the test asserting that blinded fetches weren't remembered.TestBlockRootFieldchecks that a block and its blinded form log the same root, and that a root that can't be computed logs as an error.go testforcli/...,protocol/v2/types/gloas,protocol/v2/ssv/...(spec tests included),beacon/goclientandutils/formatpasses, and so dogolangci-linton those packages andscripts/deadcode.sh.E2E passed - [action log](put link here)