Add improvements to SSZ support #403
Conversation
Co-authored-by: ltitanb <[email protected]>
Co-authored-by: Joe Clapis <[email protected]> Co-authored-by: ltitanb <[email protected]> Co-authored-by: Nils Effinghausen <[email protected]>
| let mut original_headers = req_config.headers.clone(); | ||
|
|
||
| // Check which types this request is for | ||
| let accept_types = get_accept_types(&req_config.headers).map_err(|e| { |
There was a problem hiding this comment.
why repeat this every time we send a get_header?
|
|
||
| // Send the header request | ||
| let mut start_request = Instant::now(); | ||
| let config = RequestContext { |
There was a problem hiding this comment.
if we're just cloning the full RequestContext we should just do that as it's clearer
| Ok((start_request_time, Some(get_header_response))) | ||
| } | ||
|
|
||
| async fn send_get_header_impl( |
There was a problem hiding this comment.
think would be cleaner if we try to decode the header here?
| // Get the content type; this is only really useful for OK responses, and | ||
| // doesn't handle encoding types besides SSZ and JSON | ||
| let mut content_type: Option<EncodingType> = None; | ||
| if res.status() == StatusCode::OK && |
There was a problem hiding this comment.
we can check if this is false and early return an error
| } | ||
| }; | ||
| // Regenerate the header from the response | ||
| let get_header_response = |
There was a problem hiding this comment.
in general i think we should be "forgiving" as much as possible, eg. if the header is missing we try to parse as json
|
|
||
| // Check which types this request is for | ||
| let accept_types = get_accept_types(&req_config.headers).map_err(|e| { | ||
| PbsError::GeneralRequest(format!("error reading accept types: {e}").to_string()) |
There was a problem hiding this comment.
similar here, i think we can assume json if there's any issue
| Ok(Some(block_response)) | ||
| } | ||
|
|
||
| async fn send_submit_block_impl( |
There was a problem hiding this comment.
similar here, i think we should move some of the logic here and only return a parsed block. So we submit and retry if there was an issue
|
|
||
| // This won't actually fail since the string is a const | ||
| let content_type_header = | ||
| HeaderValue::from_str(EncodingType::Ssz.content_type()).unwrap(); |
There was a problem hiding this comment.
is there a way we can make a const with this?
crates/pbs/src/routes/get_header.rs
Outdated
| let accepts_ssz = accept_types.contains(&EncodingType::Ssz); | ||
| let accepts_json = accept_types.contains(&EncodingType::Json); |
There was a problem hiding this comment.
we make this check also in the call? can we avoid the duplication eg by passing a context?
This adds a bunch of improvements to the SSZ PR (#372), including:
Still a draft until
submit_block()is done.