Implement engine_getBlobsV1
#9723
Conversation
michaelsproul
requested review from
mattsse,
Rjected,
emhane,
onbjerg and
gakonst
as code owners
crates/rpc/rpc-types/src/lib.rs
Outdated
| use serde::{Deserialize, Serialize}; | ||
|
|
||
| /// Blob type returned in responses to `engine_getBlobsV1`. | ||
| // FIXME(sproul): move to alloy? |
There was a problem hiding this comment.
yeah, but we can include it in this pr, but also add it to alloy and once we have a new release we can phase it out here
crates/rpc/rpc-types/src/lib.rs
Outdated
| /// The blob data. | ||
| pub blob: FixedBytes<BYTES_PER_BLOB>, |
There was a problem hiding this comment.
this is a bit tricky, because this can lead to stack overflow on deserialize on debug...
I encountered this here as well:
see corresponding test
so we ideally want a sanity test and perhaps we also need to do
crates/rpc/rpc-types/src/lib.rs
Outdated
| /// The blob data. | ||
| pub blob: FixedBytes<BYTES_PER_BLOB>, |
There was a problem hiding this comment.
I think we should definitely box this because we do vec![None; request.len()] so each entry even if missing will be a full blob
crates/rpc/rpc-types/Cargo.toml
Outdated
| @@ -14,6 +14,7 @@ workspace = true | |||
| [dependencies] | |||
|
|
|||
| # ethereum | |||
| alloy-eips = { workspace = true } | |||
There was a problem hiding this comment.
| alloy-eips = { workspace = true } | |
| alloy-eips.workspace |
| .inner | ||
| .tx_pool | ||
| .get_blobs_for_versioned_hashes(&versioned_hashes) | ||
| .expect("get_blobs_for_versioned_hashes is infallible")) |
There was a problem hiding this comment.
I think we can return a servererror in this case
github-actions
bot
added
the
S-stale
mattsse
added
M-prevent-stale
|
@michaelsproul sorry about the conflicts, |
|
thanks @mattsse that would be great I started on it locally but got drawn into other things and will likely continue getting distracted thanks for pushing it along! |
| versioned_hashes: &[B256], | ||
| ) -> Result<Vec<Option<BlobAndProofV1>>, BlobStoreError> { | ||
| let mut result = vec![None; versioned_hashes.len()]; | ||
| for (_tx_hash, blob_sidecar) in self.inner.store.read().iter() { |
There was a problem hiding this comment.
Whilst correct, linearly iterating the pool might be a less-than-ideal solution long run?
This is a prototype implementation of
engine_getBlobsV1, which allows the CL to fetch blobs from the EL to accelerate blob propagation:engine_getBlobsV1ethereum/execution-apis#559Before merging there are a few things that need to be resolved:
get_blobsor whether we should make it infallible (don't return a result).BlobAndProofV1type.