feat(da): support fiber (not via c-node)

[julienrbrt]

Overview

Support Fiber client (based on https://github.com/celestiaorg/celestia-app/blob/63fbf31cca216fc4e067a9e1b3a3431115c7009b/fibre), but not via celestia node or apex for this PoC
celestiaorg/celestia-node#4892

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ca79a06e-ebd4-4c19-b504-fc43188b2262

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch julien/fiber

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	⏩ skipped	✅ passed	✅ passed	Apr 13, 2026, 11:57 AM

[claude]

Claude finished @julienrbrt's task in 2m 40s —— View job

---

Code Review

• Gather context and understand the PR
• Read CLAUDE.md / AGENTS.md for repo guidelines
• Review block/internal/da/fiber_client.go
• Review block/internal/da/fiber_client_test.go
• Review block/public.go changes
• Review pkg/config/config.go changes
• Review pkg/config/config_test.go changes
• Review pkg/config/defaults.go changes
• Compile and summarize findings

---

Overview

This PR introduces a new Fiber DA backend by wrapping a FiberClient interface into the existing FullClient contract. The approach is clean in principle — adding a thin adapter layer, keeping the Fiber network interaction behind an interface, and wiring it up through config. Test coverage is thorough for the happy path. Below are the issues I found, ordered by severity.

---

Critical Issues

1. Unbounded memory growth in the local index — fiber_client.go:73

The index map[uint64][]fiberIndexedBlob accumulates every submitted blob for the lifetime of the process with no eviction, TTL, or size cap. On long-running nodes this will leak memory proportional to submission volume. Consider capping the map at a fixed number of heights (e.g. 1000) and evicting the oldest entries, or persisting the index to disk.

Fix this →

2. Retrieve ignores the namespace argument entirely — fiber_client.go:244

func (c *fiberDAClient) retrieve(_ context.Context, height uint64, _ []byte, _ bool) datypes.ResultRetrieve {

All four method parameters beyond the receiver are ignored. The local index stores blobs without namespace segregation, so Retrieve(ctx, h, headerNs) and Retrieve(ctx, h, dataNs) return the same blobs. When both namespaces are used at the same height, callers get back data for the wrong namespace. Namespace-keyed index entries (e.g. map[uint64]map[string][]fiberIndexedBlob) would fix this.

Fix this →

3. Retrieve returns StatusNotFound for data not submitted by this node

The comment on Retrieve says:

"It first checks the local submission index, then falls back to downloading via blob IDs."

But there is no fallback — it returns StatusNotFound immediately if the height isn't in the local index. This means Retrieve only works on the exact node that originally submitted the data. A syncing peer that calls Retrieve will always get StatusNotFound. Either implement the advertised fallback, or remove the misleading comment and document this limitation explicitly.

---

Architecture Concerns

4. Subscribe scans from height 0 on startup — fiber_client.go:316

var lastHeight uint64

lastHeight starts at 0, so the first poll iterates [1..currentNetworkHeight]. If the network is at height 500,000, this is a 500,000-iteration loop (mostly no-ops, but still). Initialize lastHeight from the current network height to skip already-finalized blocks:

initialHeight, err := c.fiber.GetLatestHeight(heightCtx)
if err == nil {
    lastHeight = initialHeight
}

Fix this →

5. Height re-indexing can silently corrupt downloads — fiber_client.go:203

When multiple blobs are uploaded sequentially, each upload may land at a different validator-set height (u.height). The PR canonicalizes all blobs to the last upload's height:

submitHeight := uploaded[len(uploaded)-1].height
...
id := makeFiberID(submitHeight, u.blobID)

Later, Get() calls c.fiber.Download(ctx, blobID, height) where height is extracted from the ID (i.e., submitHeight). If the Fiber network uses height as the validator-set selector for authentication, early-uploaded blobs will be fetched with the wrong validator set and may fail. Please verify with the Fiber protocol spec whether height=0 (use current head) is safe to use for all downloads, and consider replacing submitHeight with 0 if so.

---

Code Quality Issues

6. bytes.Equal vs string comparison — fiber_client.go:396, fiber_client.go:424, fiber_client.go:427

if string(b.id) == string(id) {
...
results[i] = len(proofs[i]) > 0 && string(proofs[i]) == string(b.promise)

Use bytes.Equal for byte slice comparison. The string() cast is functionally correct but idiomatic Go code uses bytes.Equal(b.id, id) to avoid any confusion about intent and prevent accidental string interning.

Fix this →

7. Potential nil-pointer dereference in NewFiberDAClient — block/public.go:74

base := da.NewFiberClient(da.FiberConfig{...})
if config.Instrumentation.IsTracingEnabled() {
    return da.WithTracingClient(base)  // base may be nil if fiberClient was nil
}
return base

da.NewFiberClient returns nil when cfg.Client == nil. If the caller passes a nil fiberClient with tracing enabled, WithTracingClient(nil) returns a tracedClient that will panic on any method call. Add a nil guard:

if base == nil {
    return nil
}

Fix this →

8. Missing Validate() for FiberDAConfig when Enabled=true

FiberDAConfig has required fields (StateAddress, KeyName) that are meaningless as empty strings when the backend is active, yet Config.Validate() doesn't call any validation on the Fiber sub-config. Add a Validate() method to FiberDAConfig and call it from the parent when Fiber.Enabled == true.

Fix this →

---

Minor / Nits

9. GetProofs returns a nil proof (not an error) for unknown IDs — fiber_client.go:388-403

If an ID is not found in the index, the corresponding proofs[i] slot is left as nil. Callers cannot distinguish "proof not available" from "empty proof". Consider returning an error for missing IDs, consistent with how Get() treats unknown IDs.

10. Subscribe includeTimestamp param is silently ignored — fiber_client.go:309

The _ bool ignores includeTimestamp. Given it's _ bool in the interface comment that states the parameter will be removed (see the interface godoc), this is acceptable as long as a comment explains the omission.

11. FiberUploadResult.BlobID comment says "typically 33 bytes" — fiber_client.go:19

// BlobID is the Fiber blob identifier (typically 33 bytes: 1 version + 32 commitment).

splitFiberID treats len(id) <= 8 as invalid, meaning a BlobID of 0 bytes would silently make splitFiberID return nil. makeFiberID and splitFiberID should either assert/validate that BlobID is non-empty, or document the minimum required length.

12. Submit with zero blobs returns StatusSuccess with empty IDs — fiber_client.go:190

This is technically correct but worth documenting; callers must check len(res.IDs) == 0 even on success. Consider adding a comment to this effect.

---

Test Coverage Gaps

• No test for Retrieve when blobs were submitted by a different fiberDAClient instance (would expose issue Transaction gossiping #3 above).
• No test verifying that Retrieve(ctx, h, nsA) does NOT return blobs submitted under nsB (would expose issue Peer discovery #2).
• No test for the Subscribe initial-height-0 edge case.
• No test for concurrent Submit + Retrieve under race conditions (recommend go test -race).

---

Summary

The implementation is a solid first pass with good interface design and test breadth. The most important issues before merging are the unbounded index memory growth (will cause OOM in production), the namespace-blind Retrieve (correctness bug), and the misleading comment + missing fallback on Retrieve (functional limitation that will confuse integrators). The nil dereference in NewFiberDAClient is a latent crash waiting to happen. The config validation gap means misconfigured Fiber nodes fail at runtime rather than startup.

[codecov]

Codecov Report

❌ Patch coverage is 90.65041% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.34%. Comparing base (cc540d5) to head (960146d).

Files with missing lines	Patch %	Lines
block/public.go	0.00%	12 Missing ⚠️
block/internal/da/fiber_client.go	95.89%	6 Missing and 3 partials ⚠️
pkg/config/config.go	75.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3244      +/-   ##
==========================================
+ Coverage   61.81%   62.34%   +0.53%     
==========================================
  Files         120      121       +1     
  Lines       12670    12916     +246     
==========================================
+ Hits         7832     8053     +221     
- Misses       3961     3982      +21     
- Partials      877      881       +4     

Flag	Coverage Δ
combined	62.34% <90.65%> (+0.53%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.