providers-alloy: BeaconClient.filtered_beacon_blobs fetches and validates only necessary blobs, and does not fallback to sidecars endpoint#3211
Conversation
Make blobs_mock mutable and clone required_query_param in the test. Add a second mock that returns garbage blob data and assert that filtered_beacon_blobs returns an error. Also fix the comment describing the blob hash.
Drop unused ReqwestProvider and BeaconBlobBundle imports, remove SIDECARS_METHOD_PREFIX_DEPRECATED, compress std imports, and delete the unused `blob_indexes` local variable.
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. |
Codecov Report❌ Patch coverage is
☔ View full report in Codecov by Sentry. |
theochap
left a comment
There was a problem hiding this comment.
Looks good! Some small rust specific comments, but the logic looks good overall
Co-authored-by: theo <80177219+theochap@users.noreply.github.com>
avoids the need to use unwrap
There was a problem hiding this comment.
Pull request overview
This PR refactors the BeaconClient.filtered_beacon_blobs method to fetch only the necessary blobs by using the beacon API's versioned_hashes query parameter, and validates the returned blobs against their computed versioned hashes using KZG commitments. This is a performance and correctness improvement over fetching all blobs for a slot.
Key changes:
- Introduces blob validation using KZG commitment verification
- Uses query parameter filtering to fetch only required blobs from the beacon API
- Removes fallback to deprecated sidecars endpoint
- Renames metrics from
BLOB_SIDECAR_*toBLOB_*for clarity
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/providers/providers-alloy/src/metrics.rs | Renamed metric constants from BLOB_SIDECAR_FETCHES/ERRORS to BLOB_FETCHES/ERRORS |
| crates/providers/providers-alloy/src/lib.rs | Exported BeaconClientError for public use |
| crates/providers/providers-alloy/src/blobs.rs | Updated metric references to use renamed constants |
| crates/providers/providers-alloy/src/beacon_client.rs | Implemented filtered blob fetching with KZG validation, added BeaconClientError type, removed deprecated sidecars endpoint fallback, added comprehensive tests |
| crates/providers/providers-alloy/Cargo.toml | Added test dependencies for httpmock and serde_json |
| Cargo.toml | Added httpmock to workspace dev-dependencies |
| Cargo.lock | Updated with new dependencies for httpmock and related crates |
Comments suppressed due to low confidence (1)
crates/providers/providers-alloy/src/beacon_client.rs:242
- This implementation causes infinite recursion. The trait method
filtered_beacon_blobs(line 234) callsself.filtered_beacon_blobs(line 242), which is the same method. This should either call the private method directly or be refactored to avoid the naming collision. The trait implementation should call the private implementation method, but they currently have the same name.
async fn filtered_beacon_blobs(
&self,
slot: u64,
blob_hashes: &[IndexedBlobHash],
) -> Result<Vec<BoxedBlobWithIndex>, BeaconClientError> {
kona_macros::inc!(gauge, Metrics::BEACON_CLIENT_REQUESTS, "method" => "blobs");
// Try to get the blobs from the blobs endpoint.
let result = self.filtered_beacon_blobs(slot, blob_hashes).await;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
BeaconClient.filtered_beacon_blobs fetches and validates only necessary blobs, and does not fallback to sidecars endpoint
…idates only necessary blobs, and does not fallback to sidecars endpoint (op-rs/kona#3211) Closes #3136 --------- Co-authored-by: theo <80177219+theochap@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Closes #3136