Skip to content

feat(config): rework FiberDAConfig for correct gRPC/JSON-RPC wiring#3285

Closed
walldiss wants to merge 1 commit intoevstack:julien/fiberfrom
walldiss:feat/fibre-config-cleanup
Closed

feat(config): rework FiberDAConfig for correct gRPC/JSON-RPC wiring#3285
walldiss wants to merge 1 commit intoevstack:julien/fiberfrom
walldiss:feat/fibre-config-cleanup

Conversation

@walldiss
Copy link
Copy Markdown

Summary

Audit of the Fiber DA config surface against how `celestia-node/api/client` actually uses the values surfaced a few problems:

  1. `state_address` is misnamed — it's the consensus gRPC endpoint, not a conceptually distinct "state" endpoint. Both `CoreGRPCConfig.Addr` (the TxClient/CoreAccessor conn) and `appfibre.Client.StateAddress` (appfibre's own state conn) target this same endpoint.
  2. There's no knob for TLS on the consensus gRPC connection — `CoreGRPCConfig.TLSEnabled` is always `false`. That breaks any deployment against a TLS-terminated consensus endpoint.
  3. Consensus gRPC and the bridge JSON-RPC usually accept different credentials (x-token vs. JWT) but `DA.AuthToken` was the single knob for both.
  4. No way to bound appfibre's FSP connection concurrency despite a dangling `// UploadConcurrency ...` comment on the struct.
  5. `bridge_address` is plain-string; operators must remember `ws://` / `wss://` or Listen silently errors with `"no out channel support"` (blob.Subscribe is channel-returning — only WebSocket works).

Changes to `FiberDAConfig`

Renamed:

  • `StateAddress` → `ConsensusAddress` (same string, clearer intent)

Added:

  • `ConsensusTLS` — drives `CoreGRPCConfig.TLSEnabled`
  • `ConsensusAuthToken` — x-token for consensus gRPC, separate from the bridge JWT
  • `BridgeAuthToken` — JWT for celestia-node bridge (`Authorization: Bearer`), separate from `DA.AuthToken`
  • `UploadConcurrency` / `DownloadConcurrency` — appfibre FSP connection caps; 0 keeps the protocol defaults

Cleanup:

  • Dropped the orphan `// UploadConcurrency ...` comment; the field now exists.

New flags (same prefix scheme)

```
evnode.da.fiber.consensus_address
evnode.da.fiber.consensus_tls
evnode.da.fiber.consensus_auth_token
evnode.da.fiber.bridge_address (pre-existing, doc updated)
evnode.da.fiber.bridge_auth_token
evnode.da.fiber.upload_concurrency
evnode.da.fiber.download_concurrency
```

Plus `evnode.da.fiber.enabled` and `evnode.da.fiber.key_name` as before.

Validation

`FiberDAConfig.Validate` surfaces misconfigs at startup instead of at first Fibre op. When `Enabled=true`:

  • `consensus_address` and `bridge_address` required
  • `bridge_address` URL must use `ws://` or `wss://` (catches the silent-break on HTTP)
  • concurrency values non-negative

Wired into `Config.Validate` alongside Raft / Pruning.

Not addressed here (called out for a follow-up)

  • `ReadConfig.HTTPHeader` passthrough. Needed for reverse-proxy auth layers (Cloudflare Access, custom API gateways). Not urgent and not a common case.
  • `EnableDATLS` field on `ReadConfig` — not a real TLS switch, only controls a warning log. TLS for the bridge is driven by the `wss://` scheme in `bridge_address`. Deliberately not exposing it.

Paired risotto change

risotto's `evolve/cmd/run.go` currently wires `CoreGRPCConfig.Addr = Fiber.BridgeAddress` — a mis-plumbing because the two fields target different physical endpoints. Once this PR lands, a paired risotto PR consumes the reworked shape, fixes the bug, and wires TLS + separate auth + concurrency.

Test plan

  • `go build ./...` clean
  • `go vet ./...` clean
  • `go test -count=1 ./pkg/config/...` passes — `TestAddFlags` flag-count assertion updated to 91 (was 84; +6 asserted flags + 1 pre-existing untested)
  • Broader `./block/... ./pkg/...` suite: four `block/internal/da` `TestFiberClient_*` failures on julien/fiber reproduce on pristine branch without this change — pre-existing, out of scope

🤖 Generated with Claude Code

Addresses an audit of the Fiber DA config surface against
celestia-node/api/client. Splits the fields so each of the two
physical endpoints — consensus gRPC and bridge JSON-RPC — has its
own address, TLS, and auth token, and exposes the appfibre
concurrency tuning that was hinted at by a dangling comment.

Field changes on FiberDAConfig:

  Renamed:
    StateAddress → ConsensusAddress
      (same string, clearer intent: this is the consensus gRPC
      endpoint, used for both the shared TxClient/CoreAccessor conn
      and appfibre.Client's state-client conn. They target the same
      host — one consensus node.)

  Added:
    ConsensusTLS        — TLS for consensus gRPC (CoreGRPCConfig.TLSEnabled)
    ConsensusAuthToken  — x-token for consensus gRPC (separate from bridge JWT)
    BridgeAuthToken     — JWT for the celestia-node bridge
                          (Authorization: Bearer). Kept distinct from the
                          existing DA.AuthToken so operators can feed
                          different credentials to each endpoint.
    UploadConcurrency   — caps concurrent FSP uploads; 0 keeps the
                          appfibre.Client protocol default.
    DownloadConcurrency — caps concurrent FSP downloads; 0 keeps the
                          appfibre.Client protocol default.

  Cleanup:
    Removed the dangling "// UploadConcurrency ..." comment that had
    no field underneath — now a real field as documented above.

Flags registered accordingly:
  evnode.da.fiber.consensus_address
  evnode.da.fiber.consensus_tls
  evnode.da.fiber.consensus_auth_token
  evnode.da.fiber.bridge_address
  evnode.da.fiber.bridge_auth_token
  evnode.da.fiber.upload_concurrency
  evnode.da.fiber.download_concurrency
  (plus the pre-existing evnode.da.fiber.enabled, evnode.da.fiber.key_name)

Validation: FiberDAConfig.Validate surfaces misconfigs at startup
rather than at first Fibre op:
  - consensus_address and bridge_address are required when enabled
  - bridge_address URL must use ws:// or wss:// scheme —
    blob.Subscribe is channel-returning and only works over
    WebSocket; plain HTTP JSON-RPC returns "no out channel support"
    and Listen silently breaks
  - non-negative concurrency values

Wired into Config.Validate alongside Raft/Pruning.

Consumer side: risotto's evolve/cmd/run.go currently wires
CoreGRPCConfig.Addr = Fiber.BridgeAddress — that's a mis-plumbing
(the two fields target different physical endpoints). A paired
risotto PR consumes the reworked shape and fixes that bug.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 24, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

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: dddcd5ed-71cf-4aac-8140-ab350fbc9625

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

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

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

@julienrbrt julienrbrt closed this Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants