-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat(anvil): add support for anvil_getBlobSidecarsByBlockId
#11828
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(anvil): add support for anvil_getBlobSidecarsByBlockId
#11828
Conversation
- Added primitive Engine REST API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, need to think about the regular http GET routes support a bit,
can we limit this pr to just the extra anvil endpoint that returns BlobTransactionSidecar
(all blobs in a block)
and think about adding support for GET requests and the beacon route in a followup pr?
crates/anvil/server/src/beacon.rs
Outdated
pub fn beacon_router<Beacon: BeaconApiHandler>(beacon: Beacon) -> Router { | ||
Router::new() | ||
.route("/v1/beacon/blob_sidecars/{block_id}", get(get_blob_sidecars_by_block_id::<Beacon>)) | ||
.with_state(beacon) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah wait, we do need this because this is a regular get request -.-
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yess it was actually less trivial than I expected 😅
no problem i'll split this PR 👌 (...tomorrow morning) |
pub fn get_blob_sidecars_by_block_id( | ||
&self, | ||
block_id: BlockId, | ||
) -> Result<Option<Vec<BlobTransactionSidecar>>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We return a Vec<BlobTransactionSidecar>
, is this what we want? or would it be better to flatten this into a single BlobTransactionSidecar
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's do just BlobTransactionSidecar
which is usually the format required
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a basic fold
will do the job as we don't have FromIterator
impl for BlobTransactionSidecar
(could be a follow up in alloy btw)
anvil_getBlobSidecarsByBlockId
anvil_getBlobSidecarsByBlockId
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
smol nit re response format
once merged we can start with the regular get endpoints
pub fn get_blob_sidecars_by_block_id( | ||
&self, | ||
block_id: BlockId, | ||
) -> Result<Option<Vec<BlobTransactionSidecar>>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's do just BlobTransactionSidecar
which is usually the format required
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, pretty cool
Motivation
towards #11797
Solution
anvil_getBlobSidecarsByBlockId
endpointPR Checklist