Use SSZ by default when calling /eth/v3/validator/blocks #7727
Conversation
| })?; | ||
| { | ||
| Ok((ssz_block_response, _)) => { | ||
| info!(slot = slot.as_u64(), "Received unsigned block in SSZ"); |
There was a problem hiding this comment.
Not sure if we need to log specifically as SSZ (and in JSON), or just leave it as the original log without in SSZ/in JSON, moving this line below and just log it after the match.
Logging in SSZ/JSON provides a bit more info but don't know if it will be helpful? Moving the log after the match and keeping it the same as current is simpler
There was a problem hiding this comment.
I don't mind either way, but I think it is redundant because of the warn! explaining the fallback to JSON.
There was a problem hiding this comment.
Makes sense, simplify in 4a3c268 (i.e., use the current info log)
michaelsproul
added
val-client
| })?; | ||
| { | ||
| Ok((ssz_block_response, _)) => { | ||
| info!(slot = slot.as_u64(), "Received unsigned block in SSZ"); |
There was a problem hiding this comment.
I don't mind either way, but I think it is redundant because of the warn! explaining the fallback to JSON.
| Err(e) => { | ||
| warn!( | ||
| slot = slot.as_u64(), | ||
| error = %e, | ||
| "Beacon node does not support SSZ in block production, falling back to JSON" |
There was a problem hiding this comment.
I'm wondering is there a way to disable attempting to use SSZ in cases where we know SSZ isn't supported? I don't think it matters too much as the overhead of the API instantly erroring should be quite small, but it's worth considering in cases where the BN -> VC latency is very high
There was a problem hiding this comment.
What we do with the builder is we track the builder's SSZ support and turn it off after the first request fails. I don't think that additional complexity is worth it here however.
#7727 introduced a bug in the logging, where as long as the node failed the SSZ `get_validator_blocks_v3` endpoint, it would log as `Beacon node does not support...`. However, the failure can be due to other reasons, such as a timed out error as found by @jimmygchen: `WARN Beacon node does not support SSZ in block production, falling back to JSON slot: 5283379, error: HttpClient(url: https://ho-h-bn-cowl.spesi.io:15052/, kind: timeout, detail: operation timed out` This PR made the error log more generic, so there is less confusion. Additionally, suggested by @michaelsproul, this PR refactors the `get_validator_blocks_v3` calls by trying all beacon nodes using the SSZ endpoint first, and if all beacon node fails the SSZ endpoint, only then fallback to JSON. It changes the logic from: "SSZ -> JSON for primary beacon node, followed by SSZ -> JSON for second beacon node and so on" to "SSZ for all beacon nodes -> JSON for all beacon nodes" This has the advantage that if the primary beacon node is having issues and failed the SSZ, we avoid retrying the primary beacon node again on JSON (as it could be that the primary beacon node fail again); rather, we switch to the second beacon node. Co-Authored-By: Tan Chee Keong <[email protected]> Co-Authored-By: chonghe <[email protected]>
3 participants
Issue Addressed
eth/v3/validator/blocks) #7698