feat: adr block stream api - #53
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds ADR-0011 for an opt-in, read-only beacon-block stream over WebSocket and gRPC. It documents fan-out, JWT authentication, configuration, event formats, limits, telemetry, and implementation requirements. It updates related ADR references and metrics, adds the ADR to the index, and revises release and upgrade information. Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/adr/0011-gateway-consumer-block-stream.md`:
- Around line 74-77: Update the ADR configuration and deployment guidance around
OPT_STREAM_ADDR, OPT_STREAM_GRPC_ADDR, and their listener setup to require TLS
or a trusted TLS-terminating proxy for any non-loopback exposure. Document that
insecure non-loopback listeners must be rejected, while preserving local
loopback development behavior and the existing authentication requirement.
- Around line 74-77: Enforce validation for the stream configuration so enabling
the consumer API with OPT_STREAM_REQUIRE_AUTH=false is rejected when listeners
bind beyond loopback, unless an explicit development mode is active. Apply this
check to the configuration parsing or startup validation symbol that handles
stream_enable, stream_require_auth, stream_addr, and stream_grpc_addr, and add
focused tests covering valid loopback/development combinations and rejected
unauthenticated public binds.
- Around line 63-80: Update the ADR’s Config section and related resource-limit
descriptions to define enforceable byte and frame limits, per-connection
rate-limit defaults, read and idle timeout values, and configuration keys where
applicable. Specify overflow behavior for events exceeding the configured
limits, including how oversized events are handled, while preserving the
existing connection and buffer settings.
- Around line 72-80: Update the configuration table entries for
OPT_STREAM_MAX_CONNS, OPT_STREAM_MAX_CONNS_PER_SUB, and OPT_STREAM_BUFFER_SIZE
so each includes its corresponding YAML key, or explicitly mark the setting as
environment-only. Keep the Env / yaml header accurate and ensure the documented
YAML configuration is not implied to work when it is ignored.
- Around line 59-60: Update the ADR’s WebSocket authentication contract near the
Authorization token requirement to define a browser-compatible bearer-token
handshake using Sec-WebSocket-Protocol, including the client offer format and
server-selected subprotocol. Ensure the JWT is used only for authentication and
is never selected or returned as the negotiated subprotocol; require
authentication before subscriber creation and the WebSocket upgrade.
- Around line 35-39: Expand the ADR’s stream contract to define a
transport-neutral event/frame union, including lagged notifications and
dropped-count semantics, and state how lag delivery remains non-blocking for
ingest. Clarify whether processBeaconBlockArrival emits once per deduplicated
payload or per source observation, then define stable block/event identity and
libp2p/mump2p correlation rules. Add focused tests covering lag delivery,
counts, deduplication, and source correlation.
In `@docs/adr/README.md`:
- Around line 22-23: Restore the ADR migration note in the README index near the
ADR listing, stating that ADR-0001–0010 contain historical and superseded
implementation details. Keep the existing ADR-0011 entry and index formatting
unchanged.
In `@docs/CHANGELOG.md`:
- Around line 67-73: Update the “Move to the current release” commands to
recreate the optimum-gateway container rather than only pulling and restarting
it. Use the deployment-specific create/run command that creates optimum-gateway
from getoptimum/gateway:v1.1.1 and passes OPT_API_KEY during container creation,
keeping the commands copy-pastable.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: getoptimum/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 30a395df-51a0-4feb-b85d-aec91cd2457f
📒 Files selected for processing (7)
README.mddocs/CHANGELOG.mddocs/adr/0001-gateway-architecture.mddocs/adr/0002-beacon-block-latency.mddocs/adr/0003-validator-metrics.mddocs/adr/0011-gateway-consumer-block-stream.mddocs/adr/README.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/adr/0002-beacon-block-latency.md (1)
88-106: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMark the retained metric descriptions as historical.
Line [93] states that these metrics no longer exist, but lines [95-106] then state in present tense that
pkg/service/telemetryprovides them. This can cause operators to copy obsolete metric names into dashboards or alerts. Change the following text to an explicitly historical description or remove it.As per path instructions, Markdown changes must prioritize technical accuracy and flag wrong metric references.
Suggested clarification
-pkg/service/telemetry` provides: +For historical context, the original 2025 design described `pkg/service/telemetry` as providing:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/adr/0002-beacon-block-latency.md` around lines 88 - 106, Mark the subsequent descriptions of block_arrival_latency_ms, eth_block_latency_ms, beacon_block_propagation_ms, and their ObserveBlockArrival/ObserveEthLatency helpers as historical, or remove them. Ensure the subsection no longer presents these obsolete metrics as currently provided by pkg/service/telemetry, while preserving the accurate current metric references above.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/adr/0002-beacon-block-latency.md`:
- Around line 88-106: Mark the subsequent descriptions of
block_arrival_latency_ms, eth_block_latency_ms, beacon_block_propagation_ms, and
their ObserveBlockArrival/ObserveEthLatency helpers as historical, or remove
them. Ensure the subsection no longer presents these obsolete metrics as
currently provided by pkg/service/telemetry, while preserving the accurate
current metric references above.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: getoptimum/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8dff6660-facb-4b3e-b296-251fb4cee6f9
📒 Files selected for processing (1)
docs/adr/0002-beacon-block-latency.md
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/CHANGELOG.md`:
- Around line 78-80: Update the Docker bind-mount arguments in the documented
command by quoting each host path containing $(pwd), including the config,
libp2p, and mump2p mounts, so the command remains copy-pastable when the
checkout path contains spaces.
- Around line 71-77: Update the Docker usage example around OPT_API_KEY to read
the credential silently without recording it in shell history, and forward it
through Docker’s environment-variable mechanism rather than embedding the value
in client arguments. Preserve the existing container configuration and
OPT_API_KEY behavior while removing the explicit exported-key and argument-based
handling.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: getoptimum/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 67ae2eab-83a8-4c84-971b-d7276adafa3b
📒 Files selected for processing (3)
docs/CHANGELOG.mddocs/adr/0011-gateway-consumer-block-stream.mddocs/adr/README.md
There was a problem hiding this comment.
Pull request overview
Adds and cross-links a new Architecture Decision Record describing a proposed (not yet implemented) consumer-facing beacon-block stream API, and refreshes project documentation to reflect current releases and clarify historical ADR details.
Changes:
- Add ADR-0011 describing an opt-in, read-only block stream over WebSocket and gRPC with auth/buffering/config/telemetry considerations.
- Update ADR index and existing ADRs to clarify “historical vs current” metric/config naming and link to ADR-0011.
- Refresh
docs/CHANGELOG.md(latest release/docs pointers, supported versions table, and upgrade guidance) and link ADRs from the root README.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Adds a documentation link to the ADR index. |
| docs/CHANGELOG.md | Updates latest release/docs, supported versions, and upgrade instructions. |
| docs/adr/README.md | Adds ADR-0011 to the ADR table and clarifies historical nature of earlier ADRs. |
| docs/adr/0011-gateway-consumer-block-stream.md | Introduces ADR-0011 detailing the proposed consumer block-stream API design. |
| docs/adr/0003-validator-metrics.md | Clarifies metric naming layers and formatting in an implementation note. |
| docs/adr/0002-beacon-block-latency.md | Adds a note about outdated metric/helper names and references current metrics location. |
| docs/adr/0001-gateway-architecture.md | Updates the consumer streaming API reference to link directly to ADR-0011. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…imum-gateway into feat/adr-block-stream-api
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/adr/0011-gateway-consumer-block-stream.md (3)
127-140: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winAlign the architecture diagram with the authentication gate.
The diagram places
AUTHafter the WebSocket and gRPC servers on the event path. Lines [79-80] require authentication before subscription and before the WebSocket upgrade. Show the consumer handshake flowing through authentication before subscription, or label authentication as a separate control path.As per path instructions, “Prioritize technical accuracy and copy-pastable commands.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/adr/0011-gateway-consumer-block-stream.md` around lines 127 - 140, The architecture diagram’s event flow incorrectly places AUTH after the WebSocket and gRPC servers. Update the Mermaid diagram around AUTH, WS, GRPC, and the consumer nodes to show authentication occurring during the consumer handshake before subscription, or clearly represent authentication as a separate control path while preserving the data-flow relationships.Source: Path instructions
112-123: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winSpecify the raw WebSocket representation.
Line [43] says WebSocket frames use JSON/text. Line [119] adds verbatim
ssz_snappybytes, but JSON/text cannot carry arbitrary bytes without an encoding. Specify base64 in a JSON field or binary WebSocket frames. Define the exact decoded-byte guarantee and the meaning ofblock_size_bytes.As per path instructions, “Prioritize technical accuracy and copy-pastable commands.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/adr/0011-gateway-consumer-block-stream.md` around lines 112 - 123, Clarify the raw BlockEvent WebSocket representation by choosing either base64-encoded ssz_snappy in a JSON field or binary WebSocket frames, and document the exact decoded-byte guarantee for consumers. Define whether block_size_bytes measures the original SSZ payload or the compressed ssz_snappy bytes, keeping metadata and raw mode semantics consistent.Source: Path instructions
59-64: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDefine
SubscribeRequestand transport parity.Line [59] defines
modeandtopicsonly for WebSocket. The ADR does not define the fields, defaults, or validation behavior ofSubscribeRequest. Specify how gRPC selectsmetadataorraw, requestsbeacon_block, and rejects invalid values. Keep both transports on the same request contract.As per coding guidelines, “Flag changed paths that may silently alter semantics or break invariants.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/adr/0011-gateway-consumer-block-stream.md` around lines 59 - 64, Update the ADR’s gRPC section and SubscribeRequest definition to specify the shared request contract: mode must support metadata or raw with an explicit default, topics must request beacon_block with its default and validation, and invalid values must be rejected. Ensure the WebSocket and gRPC transports use identical field semantics, defaults, and validation behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/adr/0011-gateway-consumer-block-stream.md`:
- Around line 127-140: The architecture diagram’s event flow incorrectly places
AUTH after the WebSocket and gRPC servers. Update the Mermaid diagram around
AUTH, WS, GRPC, and the consumer nodes to show authentication occurring during
the consumer handshake before subscription, or clearly represent authentication
as a separate control path while preserving the data-flow relationships.
- Around line 112-123: Clarify the raw BlockEvent WebSocket representation by
choosing either base64-encoded ssz_snappy in a JSON field or binary WebSocket
frames, and document the exact decoded-byte guarantee for consumers. Define
whether block_size_bytes measures the original SSZ payload or the compressed
ssz_snappy bytes, keeping metadata and raw mode semantics consistent.
- Around line 59-64: Update the ADR’s gRPC section and SubscribeRequest
definition to specify the shared request contract: mode must support metadata or
raw with an explicit default, topics must request beacon_block with its default
and validation, and invalid values must be rejected. Ensure the WebSocket and
gRPC transports use identical field semantics, defaults, and validation
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: getoptimum/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 792aa328-1cbf-4e6d-ab4c-98de755485b2
📒 Files selected for processing (1)
docs/adr/0011-gateway-consumer-block-stream.md
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
13-13: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove or explain unrelated legal-text edits.
The README change is scoped to adding the ADR index link, but these lines also change license and patent notices. Remove them from this PR or document and validate their intended legal effect.
Also applies to: 217-225
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 13, Revert the unrelated license and patent notice changes in README.md, preserving only the intended ADR index link. If those legal-text edits are required, document their purpose and validate their legal effect before retaining them.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/adr/0002-beacon-block-latency.md`:
- Around line 90-91: Update the histogram descriptions in the ADR around
RecordBlockPathArrival and the block_arrival_libp2p_ms/block_arrival_mump2p_ms
entries to describe per-source arrivals, replacing the “for a block first seen
via” wording. Keep the separate blocks_first_seen_* counter descriptions
unchanged.
- Line 175: Update the “Destination arrival timestamps” section to remove
current guidance referencing the deleted ObserveBlockArrival and
ObserveEthLatency APIs. Describe the existing RecordBlockPathArrival flow
instead, or explicitly label the section as historical, while preserving
consistency with the removal noted earlier in the document.
---
Outside diff comments:
In `@README.md`:
- Line 13: Revert the unrelated license and patent notice changes in README.md,
preserving only the intended ADR index link. If those legal-text edits are
required, document their purpose and validate their legal effect before
retaining them.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: getoptimum/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 15ccabf0-67f9-4210-a250-0cb4472e856a
📒 Files selected for processing (2)
README.mddocs/adr/0002-beacon-block-latency.md
Summary by CodeRabbit