refactor(blobber): abstract blob sources and remove reth deps by prestwich · Pull Request #112 · init4tech/node-components

prestwich · 2026-03-20T16:43:03Z

[Claude Code]

Summary

Define BlobSource (sync) and AsyncBlobSource (async) traits in signet-blobber
Make BlobFetcher non-generic — holds Vec<Box<dyn BlobSource>> and Vec<Box<dyn AsyncBlobSource>>
Extract BlobExplorerSource, BeaconBlobSource, PylonBlobSource as standalone AsyncBlobSource impls
Replace reth::network::cache::LruMap with direct schnellru dep
Replace reth::rpc::types::beacon::sidecar::GetBlobsResponse with alloy import
Replace BlobStoreError with Box<dyn Error + Send + Sync>
Add MemoryBlobSource behind test-utils feature for in-memory blob testing
Move RecoveredBlockShim from blobber to signet-host-reth
Add RethBlobSource<P> newtype in signet-host-reth (orphan rule wrapper)
Remove reth, reth-chainspec, reth-transaction-pool from signet-blobber
Remove reth, reth-exex-test-utils from signet-node-tests

PR stack

Merge in order:

feat: add signet-host-reth crate with RethHostNotifier #106 ← signet-host-reth

feat: add signet-host-rpc crate with RpcHostNotifier #107 ← signet-host-rpc

fix(node-tests): adapt test infra to HostNotifier trait #108 ← test infra cleanup (base of this PR)

this PR ← blobber abstraction

Linear

ENG-2077 (child of ENG-2056)

Test plan

🤖 Generated with Claude Code

Review Fixes

Add tracing::debug! events for sync and async blob source errors in fetch_blobs
Guard against empty blob responses: sources return Ok(None) instead of Ok(Some(empty))
Fix stale comment referencing tokio::select (code uses select_all)
Merge split use alloy:: imports in rpc_debug.rs
Consolidate BlobFuture/BlobSourceError type aliases into source.rs as pub(crate)
Add .error_for_status()? to beacon and pylon HTTP requests
Remove unused BuilderError::Client variant
Rename Blobs::FromPool → Blobs::FromSidecar

prestwich · 2026-03-20T17:55:34Z

Code review

Found 1 issue:

Blob source errors silently swallowed in BlobFetcher::fetch_blobs with no tracing. Both sync source errors (Err(_) => continue) and async source errors (discarded by the if let Ok(Some(blobs)) pattern) produce no diagnostic output. Network failures, deserialization errors, and HTTP errors from any source are invisible. CLAUDE.md says "ALWAYS use tracing for instrumentation" and "Use events to represent discrete occurrences within a span, like errors or significant state changes." WARN level is appropriate here per the guideline: "ignorable errors that may indicate a problem."

node-components/crates/blobber/src/blobs/fetch.rs

Lines 136 to 158 in f138e32

    
           // Try each sync source in order 
        
           for source in &self.sync_sources { 
        
               match source.get_blob(spec) { 
        
                   Ok(Some(blobs)) => return Ok(blobs), 
        
                   Ok(None) | Err(_) => continue, 
        
               } 
        
           } 
        
           // Race all async sources concurrently using tokio::select on 
        
           // pinned futures (all borrowed from &self, no spawning needed). 
        
           if !self.async_sources.is_empty() { 
        
               let mut futs: Vec<_> = self.async_sources.iter().map(|s| s.get_blob(spec)).collect(); 
        
               while !futs.is_empty() { 
        
                   let (result, _index, remaining) = futures_util::future::select_all(futs).await; 
        
                   if let Ok(Some(blobs)) = result { 
        
                       return Ok(blobs); 
        
                   } 
        
                   futs = remaining; 
        
               } 
        
           } 
        
           Err(FetchError::MissingSidecar(spec.tx_hash))

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…cies - Extract BlobExplorerSource, BeaconBlobSource, PylonBlobSource as AsyncBlobSource impls - Make BlobFetcher non-generic with trait object sources - Replace LruMap with schnellru, GetBlobsResponse with alloy import - Replace BlobStoreError with boxed error - Remove dead FetchError variants (ConsensusClientUrlNotSet, PylonClientUrlNotSet, BlobCountMismatch) - Delete shim.rs (RecoveredBlockShim moves to host-reth in later task) - Remove reth, reth-chainspec, reth-transaction-pool dependencies Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…ug.rs Replace reth type re-exports with signet-types and alloy equivalents. Remove reth from Cargo.toml entirely. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…lobs - Log source errors at DEBUG level in fetch_blobs (errors only matter if ALL sources fail) - Return Ok(None) instead of Ok(Some(empty)) when sources return zero blobs, so the fetcher continues to the next source - Fix stale comment referencing tokio::select (uses select_all) - Merge split alloy imports in rpc_debug.rs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Fraser999 · 2026-03-23T14:42:04Z

crates/blobber/src/blobs/source.rs

+/// Boxed error type for blob source operations.
+type BlobSourceError = Box<dyn core::error::Error + Send + Sync>;
+
+/// Boxed future returned by [`AsyncBlobSource::get_blob`].
+type BlobFuture<'a> =
+    Pin<Box<dyn Future<Output = Result<Option<Blobs>, BlobSourceError>> + Send + 'a>>;


Should we make these pub(crate) so they can be reused? (they're defined identically in a handful of files)

[Claude Code]

Done — made them pub(crate) in source.rs and imported from the three concrete source files.

Fraser999 · 2026-03-23T14:55:26Z

crates/blobber/src/blobs/sources/beacon.rs

+    let hashes = versioned_hashes.iter().map(|h| h.to_string()).collect::<Vec<_>>().join(",");
+    url.query_pairs_mut().append_pair("versioned_hashes", &hashes);
+
+    let response = client.get(url).header("accept", "application/json").send().await?;


Suggested change

let response = client.get(url).header("accept", "application/json").send().await?;

let response = client.get(url).header("accept", "application/json").send().await?.error_for_status()?;

[Claude Code]

Done.

Fraser999 · 2026-03-23T14:56:03Z

crates/blobber/src/blobs/sources/pylon.rs

+) -> Result<Blobs, BlobSourceError> {
+    let url = base_url.join(&format!("sidecar/{tx_hash}"))?;
+
+    let response = client.get(url).header("accept", "application/json").send().await?;


Suggested change

let response = client.get(url).header("accept", "application/json").send().await?;

let response = client.get(url).header("accept", "application/json").send().await?.error_for_status()?;

[Claude Code]

Done.

Fraser999 · 2026-03-23T15:03:03Z

crates/blobber/src/blobs/builder.rs

    /// The client failed to build.
    #[error("failed to build client: {0}")]
    Client(#[from] reqwest::Error),


We don't use this variant any more either.

[Claude Code]

Removed. Added Copy/Clone derives to BuilderError since it is now single-variant.

Fraser999 · 2026-03-23T15:05:05Z

crates/blobber/src/blobs/fetch.rs

Maybe Blobs::FromPool should be renamed to Blobs::FromSidecar?

[Claude Code]

Renamed to Blobs::FromSidecar — fits better now that non-pool sources like MemoryBlobSource also produce this variant.

- Consolidate BlobFuture/BlobSourceError type aliases into source.rs as pub(crate) and import from concrete source files - Add .error_for_status()? to beacon and pylon HTTP requests - Remove unused BuilderError::Client variant - Rename Blobs::FromPool to Blobs::FromSidecar Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

prestwich requested a review from a team as a code owner March 20, 2026 16:43

prestwich mentioned this pull request Mar 20, 2026

refactor(node-config): remove reth deps, add FromEnv to RPC configs #113

Merged

7 tasks

prestwich marked this pull request as draft March 20, 2026 18:19

prestwich force-pushed the fix/node-tests-host-notifier branch from b21848c to b2fd16d Compare March 20, 2026 18:22

prestwich force-pushed the prestwich/eng-2077-blobber-abstraction branch from 1cd7a2b to 8e7eca9 Compare March 20, 2026 18:23

prestwich mentioned this pull request Mar 20, 2026

feat(host-rpc): add RpcAliasOracle with shared positive-result cache #114

Merged

3 tasks

prestwich and others added 7 commits March 20, 2026 14:49

feat(blobber): add BlobSource and AsyncBlobSource traits

e512453

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

feat(blobber): add MemoryBlobSource for testing

ddeee9d

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

feat(host-reth): add RethBlobSource and absorb RecoveredBlockShim

18c4f71

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

refactor(node-tests): replace reth pool with MemoryBlobSource

0819eac

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

fix(node-tests): remove remaining reth deps from utils.rs and rpc_deb…

d6c3594

…ug.rs Replace reth type re-exports with signet-types and alloy equivalents. Remove reth from Cargo.toml entirely. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

prestwich force-pushed the prestwich/eng-2077-blobber-abstraction branch from 8e7eca9 to 84ba68b Compare March 20, 2026 18:50

prestwich changed the base branch from fix/node-tests-host-notifier to develop March 20, 2026 18:50

prestwich marked this pull request as ready for review March 20, 2026 18:50

Fraser999 reviewed Mar 23, 2026

View reviewed changes

Fraser999 approved these changes Mar 23, 2026

View reviewed changes

prestwich merged commit 3935687 into develop Mar 23, 2026
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(blobber): abstract blob sources and remove reth deps#112

refactor(blobber): abstract blob sources and remove reth deps#112
prestwich merged 8 commits intodevelopfrom
prestwich/eng-2077-blobber-abstraction

prestwich commented Mar 20, 2026 •

edited

Loading

Uh oh!

prestwich commented Mar 20, 2026

Uh oh!

Fraser999 Mar 23, 2026

Uh oh!

prestwich Mar 23, 2026

Uh oh!

Fraser999 Mar 23, 2026

Uh oh!

prestwich Mar 23, 2026

Uh oh!

Fraser999 Mar 23, 2026

Uh oh!

prestwich Mar 23, 2026

Uh oh!

Fraser999 Mar 23, 2026

Uh oh!

prestwich Mar 23, 2026

Uh oh!

Fraser999 Mar 23, 2026

Uh oh!

prestwich Mar 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	let response = client.get(url).header("accept", "application/json").send().await?;
	let response = client.get(url).header("accept", "application/json").send().await?.error_for_status()?;

Conversation

prestwich commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

PR stack

Linear

Test plan

Review Fixes

Uh oh!

prestwich commented Mar 20, 2026

Code review

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

prestwich commented Mar 20, 2026 •

edited

Loading