Implement bootnode health checks and related configurations#22
Open
anunay-xin wants to merge 2 commits into
Open
Implement bootnode health checks and related configurations#22anunay-xin wants to merge 2 commits into
anunay-xin wants to merge 2 commits into
Conversation
- Added environment variables for bootnode health checks in `docker-compose.yml`. - Updated `start-wizard.sh` to export bootnode network configuration. - Enhanced `main.go` to initialize bootnode health checking if enabled. - Introduced new API endpoints for bootnode health status and checks. - Updated README with bootnode health configuration details. - Modified Go modules to include necessary dependencies for bootnode functionality.
📝 WalkthroughWalkthroughAdds end-to-end bootnode health monitoring: a discv4 UDP ping client, an RLPx TCP dial client, a probe aggregation model, a periodic ChangesBootnode Health Monitoring
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser
participant useBootnodePolling as useBootnodePolling
participant API as GET /v2/bootnodes/health
participant BootnodeChecker as BootnodeChecker
participant discv4Client as discv4.Client (UDP)
participant rlpxClient as rlpx.Client (TCP)
Browser->>useBootnodePolling: mount / interval tick
useBootnodePolling->>API: GET /v2/bootnodes/health
API->>BootnodeChecker: Snapshot()
BootnodeChecker-->>API: BootnodeHealthReport
API-->>useBootnodePolling: 200 JSON
useBootnodePolling-->>Browser: report state update → BootnodesPanel render
Browser->>useBootnodePolling: onCheckNow click
useBootnodePolling->>API: POST /v2/bootnodes/check (x-api-secret)
API->>BootnodeChecker: CheckNow() async
BootnodeChecker->>discv4Client: Ping(node) × N workers
BootnodeChecker->>rlpxClient: Dial(node, timeout) × N workers
discv4Client-->>BootnodeChecker: RTT / ErrTimeout
rlpxClient-->>BootnodeChecker: DialOutcome
BootnodeChecker->>BootnodeChecker: storeReport(snapshot)
useBootnodePolling->>API: poll GET /v2/bootnodes/health until checkedAt changes
API-->>useBootnodePolling: updated BootnodeHealthReport
useBootnodePolling-->>Browser: checking=false, updated panel
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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. 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: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/README.md (1)
18-18: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUpdate Go prerequisite to match bumped toolchain.
README lists
Go 1.23+butDockerfileandgo.modwere bumped to1.25. Align the prerequisite toGo 1.25+to prevent developer environment mismatches.📝 Suggested fix
- - Go 1.23+ + - Go 1.25+🤖 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 `@backend/README.md` at line 18, The prerequisite listed in the README is out of sync with the bumped Go toolchain. Update the Go version requirement in the README so it matches the version used by the project’s toolchain and module settings, aligning it with the `Dockerfile` and `go.mod` changes to `Go 1.25+`.
🤖 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 `@backend/internal/api/bootnodes.go`:
- Around line 22-24: The BootnodeHandler.Health endpoint currently always
returns 200 with h.checker.Snapshot(), even when the checker has not produced a
real report and Snapshot() is still zero-valued. Update Health to detect whether
a successful snapshot exists from BootnodeChecker/checkAll state and return 503
(or an explicit error response) until a valid report is available, using the
BootnodeHandler and checker snapshot flow as the place to enforce the failure
state.
- Around line 39-45: The BootnodeHandler.checkAuth path currently allows POST
/v2/bootnodes/check when adminSecret is empty, which leaves the admin-only check
endpoint exposed. Update checkAuth to reject requests when h.adminSecret is
unset (return a 403/503-style failure instead of true), or gate the
BootnodesCheck route registration so it is not wired without a configured
secret. Use BootnodeHandler, checkAuth, and BootnodesCheck to locate the fix.
In `@backend/internal/discv4/client_integration_test.go`:
- Around line 5-22: Update the integration test to match the current Client.Ping
signature: in TestPingLiveBootnode, stop passing the timeout argument and call
Ping with only the *Node returned by ParseNode. Use the Client.Ping method in
backend/internal/discv4/client.go as the reference for the expected call shape,
and keep the rest of the test flow unchanged so it compiles under the
integration tag.
In `@backend/internal/discv4/client.go`:
- Around line 56-70: `BOOTNODE_TIMEOUT` is still not being applied to discv4
ping probes because `Client.Ping` in `backend/internal/discv4/client.go` uses
the discovery stack’s fixed timeout path. Update the `Ping` API (and its call
sites such as the UDP path from `bootnode_checker.go`) to accept and honor a
caller-provided timeout, and make sure the underlying `c.tab.Ping`/pong wait
uses that value instead of the built-in 500 ms limit.
In `@backend/internal/discv4/node.go`:
- Around line 26-35: `Node.Endpoint()` and `Node.TCPEndpoint()` are rendering
host:port manually, which breaks IPv6 formatting; update both methods to use
`net.JoinHostPort` so `Node` emits bracketed IPv6 endpoints consistently. Keep
the existing TCP-vs-UDP selection logic in `Endpoint()`, but change the final
string construction in these methods so callers like `bootnode_checker` receive
valid endpoint strings for both IPv4 and IPv6.
In `@backend/internal/rlpx/classify_test.go`:
- Around line 9-33: The TestClassifyDial table is not asserting the expected
error text because the contains field is never checked, so wrong disconnect
messages can still pass. Update TestClassifyDial to validate classifyDial’s
Message against each row’s contains value for the non-empty cases, using the
classifyDial helper and the existing tt.contains column to ensure the returned
substring matches the expected disconnect reason.
In `@backend/internal/service/bootnode_checker.go`:
- Around line 205-258: The probe helpers are not respecting the configured
timeout as a real per-node budget: probeUDP ignores timeout entirely, and
probeTCP can exceed it because the minimum 1-second floor is applied per
attempt. Update probeUDP and probeTCP in bootnode_checker.go so the timeout is
divided across attempts and enforced as a total budget, with backoff/counting
included, while keeping the existing attempt aggregation via probe.Aggregate and
the local-init fast paths in workerClients.
- Around line 126-136: `checkAll()` leaves `c.report` unchanged when
`loadNodes()` fails or returns no nodes, so `Snapshot()` keeps serving stale
health data. Update `checkAll()` to explicitly clear or replace the current
snapshot in the `loadNodes()` error and empty-list branches, using the existing
`c.report`/`Snapshot()` flow so `GET /v2/bootnodes/health` reflects the current
failure or empty state instead of the previous success.
In `@backend/internal/service/bootnode_load.go`:
- Around line 37-48: `LoadBootnodes` is re-fetching bootnode data on every
health check, which ties each probe cycle to `discv4.LoadNodesFromURL` and makes
transient network/GitHub failures abort checks. Update `LoadBootnodes` and the
caller in `bootnode_checker` to use a cached parsed bootnode list, refreshing it
on startup or behind a long TTL instead of downloading on every interval. Keep
the network-specific selection logic (`BootnodesURLForNetwork`,
`loadDevnetBootnodes`) but decouple it from per-check fetching.
In `@frontend/src/App.tsx`:
- Around line 162-172: Bootnode polling is being disabled by the
Boolean(API_URL) gate in App’s useBootnodePolling setup, which blocks the
default same-origin case where API_URL falls back to ''. Update the App
component so useBootnodePolling still runs when API_URL is empty/same-origin,
and only disable it for truly invalid or intentionally off configurations. Keep
the existing API_URL handling consistent with the rest of the file and preserve
the polling flow for /v2/bootnodes/* in the default setup.
In `@frontend/src/components/BootnodesPanel.tsx`:
- Around line 331-344: The new action buttons in BootnodesPanel remove the
native focus outline with focus:outline-none but do not provide a replacement,
so keyboard focus becomes invisible. Update the affected button styles in the
copy/close controls and the other button groups in this component (the modal,
error, collapse, and pagination actions) to use a visible focus state, such as a
focus ring or outlined border, consistent with the existing button styling.
- Around line 759-769: The sortable table headers in BootnodesPanel are only
clickable on the <th>, which makes them unreachable by keyboard. Move the
sorting action into a वास्तविक button inside the header cell in the
BOOTNODE_COLUMNS render loop, and keep the tooltip behavior on the cell if
needed. Make the button focusable/activatable by keyboard and expose the current
sort state with aria-sort on the corresponding header based on the active sort
key and direction. Ensure handleSort and SortIcon still drive the same sorting
logic and visual state.
---
Outside diff comments:
In `@backend/README.md`:
- Line 18: The prerequisite listed in the README is out of sync with the bumped
Go toolchain. Update the Go version requirement in the README so it matches the
version used by the project’s toolchain and module settings, aligning it with
the `Dockerfile` and `go.mod` changes to `Go 1.25+`.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 14be5205-1349-4bef-8c1e-f96c2f9176ec
⛔ Files ignored due to path filters (1)
backend/go.sumis excluded by!**/*.sum
📒 Files selected for processing (28)
backend/.env_samplebackend/Dockerfilebackend/README.mdbackend/config/config.gobackend/go.modbackend/internal/api/bootnodes.gobackend/internal/api/handlers.gobackend/internal/api/router.gobackend/internal/discv4/client.gobackend/internal/discv4/client_integration_test.gobackend/internal/discv4/load.gobackend/internal/discv4/node.gobackend/internal/discv4/node_test.gobackend/internal/probe/probe.gobackend/internal/probe/probe_test.gobackend/internal/rlpx/classify.gobackend/internal/rlpx/classify_test.gobackend/internal/rlpx/client.gobackend/internal/service/bootnode_checker.gobackend/internal/service/bootnode_load.gobackend/internal/service/bootnode_load_test.gobackend/main.godocker-compose.ymlfrontend/src/App.tsxfrontend/src/components/BootnodesPanel.tsxfrontend/src/hooks/useBootnodePolling.tsfrontend/src/types.tsstart-wizard.sh
Comment on lines
+22
to
+24
| func (h *BootnodeHandler) Health(c *gin.Context) { | ||
| c.Header("Cache-Control", "no-store") | ||
| c.JSON(http.StatusOK, h.checker.Snapshot()) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Return a failure state until the checker has produced a real report.
checkAll() exits without storing a report when bootnode loading fails or no nodes are available, so Snapshot() stays zero-valued. This handler still answers 200, which makes clients treat the feature as available and render an empty result instead of the disabled/error path. Please return 503 (or an explicit error payload) until a successful snapshot exists.
🤖 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 `@backend/internal/api/bootnodes.go` around lines 22 - 24, The
BootnodeHandler.Health endpoint currently always returns 200 with
h.checker.Snapshot(), even when the checker has not produced a real report and
Snapshot() is still zero-valued. Update Health to detect whether a successful
snapshot exists from BootnodeChecker/checkAll state and return 503 (or an
explicit error response) until a valid report is available, using the
BootnodeHandler and checker snapshot flow as the place to enforce the failure
state.
Comment on lines
+39
to
+45
| func (h *BootnodeHandler) checkAuth(c *gin.Context) bool { | ||
| if h.adminSecret == "" { | ||
| return true | ||
| } | ||
| secret := c.GetHeader("x-api-secret") | ||
| if secret != h.adminSecret { | ||
| c.JSON(http.StatusUnauthorized, gin.H{ |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Keep the manual check endpoint closed when no admin secret is configured.
This is labeled admin-only, but checkAuth returns true when ADMIN_SECRET is unset. That leaves POST /v2/bootnodes/check callable by anyone, and each request can trigger outbound UDP/TCP probes. Return 403/503 here when no admin secret is configured, or avoid wiring BootnodesCheck at all in that case.
Possible fix
func (h *BootnodeHandler) checkAuth(c *gin.Context) bool {
if h.adminSecret == "" {
- return true
+ c.JSON(http.StatusServiceUnavailable, gin.H{
+ "error": "admin secret not configured",
+ "message": "manual bootnode checks are disabled until ADMIN_SECRET is set",
+ })
+ return false
}
secret := c.GetHeader("x-api-secret")
if secret != h.adminSecret {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func (h *BootnodeHandler) checkAuth(c *gin.Context) bool { | |
| if h.adminSecret == "" { | |
| return true | |
| } | |
| secret := c.GetHeader("x-api-secret") | |
| if secret != h.adminSecret { | |
| c.JSON(http.StatusUnauthorized, gin.H{ | |
| func (h *BootnodeHandler) checkAuth(c *gin.Context) bool { | |
| if h.adminSecret == "" { | |
| c.JSON(http.StatusServiceUnavailable, gin.H{ | |
| "error": "admin secret not configured", | |
| "message": "manual bootnode checks are disabled until ADMIN_SECRET is set", | |
| }) | |
| return false | |
| } | |
| secret := c.GetHeader("x-api-secret") | |
| if secret != h.adminSecret { | |
| c.JSON(http.StatusUnauthorized, gin.H{ |
🤖 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 `@backend/internal/api/bootnodes.go` around lines 39 - 45, The
BootnodeHandler.checkAuth path currently allows POST /v2/bootnodes/check when
adminSecret is empty, which leaves the admin-only check endpoint exposed. Update
checkAuth to reject requests when h.adminSecret is unset (return a 403/503-style
failure instead of true), or gate the BootnodesCheck route registration so it is
not wired without a configured secret. Use BootnodeHandler, checkAuth, and
BootnodesCheck to locate the fix.
Comment on lines
+5
to
+22
| import ( | ||
| "testing" | ||
| "time" | ||
| ) | ||
|
|
||
| func TestPingLiveBootnode(t *testing.T) { | ||
| enode := "enode://efc710050d65cda5d563fc6c9520986a69bbed81036b427a5ad7a2079bc8ba421e78d291a9388a1bd10ff5d9afdc2a095900528f5c45942104d818feda37cf7a@154.12.116.170:30301" | ||
| n, err := ParseNode(enode) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| client, err := NewClient() | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| defer client.Close() | ||
|
|
||
| rtt, err := client.Ping(n, 5*time.Second) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Update the integration test to the current Ping signature.
Client.Ping in backend/internal/discv4/client.go:58-70 only accepts *Node, so this file no longer compiles under go test -tags=integration.
Suggested fix
import (
"testing"
- "time"
)
@@
- rtt, err := client.Ping(n, 5*time.Second)
+ rtt, err := client.Ping(n)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import ( | |
| "testing" | |
| "time" | |
| ) | |
| func TestPingLiveBootnode(t *testing.T) { | |
| enode := "enode://efc710050d65cda5d563fc6c9520986a69bbed81036b427a5ad7a2079bc8ba421e78d291a9388a1bd10ff5d9afdc2a095900528f5c45942104d818feda37cf7a@154.12.116.170:30301" | |
| n, err := ParseNode(enode) | |
| if err != nil { | |
| t.Fatal(err) | |
| } | |
| client, err := NewClient() | |
| if err != nil { | |
| t.Fatal(err) | |
| } | |
| defer client.Close() | |
| rtt, err := client.Ping(n, 5*time.Second) | |
| import ( | |
| "testing" | |
| ) | |
| func TestPingLiveBootnode(t *testing.T) { | |
| enode := "enode://efc710050d65cda5d563fc6c9520986a69bbed81036b427a5ad7a2079bc8ba421e78d291a9388a1bd10ff5d9afdc2a095900528f5c45942104d818feda37cf7a@154.12.116.170:30301" | |
| n, err := ParseNode(enode) | |
| if err != nil { | |
| t.Fatal(err) | |
| } | |
| client, err := NewClient() | |
| if err != nil { | |
| t.Fatal(err) | |
| } | |
| defer client.Close() | |
| rtt, err := client.Ping(n) |
🤖 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 `@backend/internal/discv4/client_integration_test.go` around lines 5 - 22,
Update the integration test to match the current Client.Ping signature: in
TestPingLiveBootnode, stop passing the timeout argument and call Ping with only
the *Node returned by ParseNode. Use the Client.Ping method in
backend/internal/discv4/client.go as the reference for the expected call shape,
and keep the rest of the test flow unchanged so it compiles under the
integration tag.
Comment on lines
+56
to
+70
| // Ping sends a single pingXDC to n and returns the RTT on success. | ||
| // The internal discovery timeout (500 ms) governs how long to wait for a pong. | ||
| func (c *Client) Ping(n *Node) (time.Duration, error) { | ||
| if err := n.validate(); err != nil { | ||
| return 0, err | ||
| } | ||
| target, err := enode.ParseV4(n.Enode()) | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
| start := time.Now() | ||
| if err := c.tab.Ping(target); err != nil { | ||
| return 0, err | ||
| } | ||
| return time.Since(start), nil |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
BOOTNODE_TIMEOUT is still ignored by discv4 probes. backend/internal/service/bootnode_checker.go:205-223 passes a timeout through the UDP path, but backend/internal/discv4/client.go only exposes Ping(n *Node) and still relies on the discovery stack’s fixed internal timeout. That leaves UDP health checks capped at the old limit even when the configured timeout is higher.
🤖 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 `@backend/internal/discv4/client.go` around lines 56 - 70, `BOOTNODE_TIMEOUT`
is still not being applied to discv4 ping probes because `Client.Ping` in
`backend/internal/discv4/client.go` uses the discovery stack’s fixed timeout
path. Update the `Ping` API (and its call sites such as the UDP path from
`bootnode_checker.go`) to accept and honor a caller-provided timeout, and make
sure the underlying `c.tab.Ping`/pong wait uses that value instead of the
built-in 500 ms limit.
Comment on lines
+26
to
+35
| func (n *Node) Endpoint() string { | ||
| if n.UDP == n.TCP { | ||
| return fmt.Sprintf("%s:%d", n.IP, n.TCP) | ||
| } | ||
| return fmt.Sprintf("%s:%d", n.IP, n.UDP) | ||
| } | ||
|
|
||
| func (n *Node) TCPEndpoint() string { | ||
| return fmt.Sprintf("%s:%d", n.IP, n.TCP) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use net.JoinHostPort for endpoint rendering.
Endpoint() and TCPEndpoint() currently build host:port with %s:%d, so an IPv6 node becomes 2001:db8::1:30303 instead of [2001:db8::1]:30303. ParseNode() and Enode() already handle IPv6, and backend/internal/service/bootnode_checker.go:165-172 surfaces these strings directly, so any IPv6 bootnode will be exposed with a broken endpoint.
Suggested fix
func (n *Node) Endpoint() string {
- if n.UDP == n.TCP {
- return fmt.Sprintf("%s:%d", n.IP, n.TCP)
- }
- return fmt.Sprintf("%s:%d", n.IP, n.UDP)
+ port := n.UDP
+ if n.UDP == n.TCP {
+ port = n.TCP
+ }
+ return net.JoinHostPort(n.IP.String(), strconv.Itoa(int(port)))
}
func (n *Node) TCPEndpoint() string {
- return fmt.Sprintf("%s:%d", n.IP, n.TCP)
+ return net.JoinHostPort(n.IP.String(), strconv.Itoa(int(n.TCP)))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func (n *Node) Endpoint() string { | |
| if n.UDP == n.TCP { | |
| return fmt.Sprintf("%s:%d", n.IP, n.TCP) | |
| } | |
| return fmt.Sprintf("%s:%d", n.IP, n.UDP) | |
| } | |
| func (n *Node) TCPEndpoint() string { | |
| return fmt.Sprintf("%s:%d", n.IP, n.TCP) | |
| } | |
| func (n *Node) Endpoint() string { | |
| port := n.UDP | |
| if n.UDP == n.TCP { | |
| port = n.TCP | |
| } | |
| return net.JoinHostPort(n.IP.String(), strconv.Itoa(int(port))) | |
| } | |
| func (n *Node) TCPEndpoint() string { | |
| return net.JoinHostPort(n.IP.String(), strconv.Itoa(int(n.TCP))) | |
| } |
🤖 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 `@backend/internal/discv4/node.go` around lines 26 - 35, `Node.Endpoint()` and
`Node.TCPEndpoint()` are rendering host:port manually, which breaks IPv6
formatting; update both methods to use `net.JoinHostPort` so `Node` emits
bracketed IPv6 endpoints consistently. Keep the existing TCP-vs-UDP selection
logic in `Endpoint()`, but change the final string construction in these methods
so callers like `bootnode_checker` receive valid endpoint strings for both IPv4
and IPv6.
Comment on lines
+205
to
+258
| func probeUDP(wc workerClients, n *discv4.Node, timeout time.Duration, attempts, minSuccess int) probe.Summary { | ||
| if wc.udpInit != nil { | ||
| return probe.LocalSummary(wc.udpInit) | ||
| } | ||
|
|
||
| tries := make([]probe.Attempt, 0, attempts) | ||
| for i := 0; i < attempts; i++ { | ||
| if i > 0 { | ||
| time.Sleep(150 * time.Millisecond) | ||
| } | ||
| rtt, err := wc.udp.Ping(n) | ||
| if err == nil { | ||
| tries = append(tries, probe.Attempt{Outcome: probe.OutcomeOK, RTT: rtt}) | ||
| continue | ||
| } | ||
| tries = append(tries, probe.Attempt{Outcome: probe.OutcomeFail, Message: err.Error()}) | ||
| } | ||
| return probe.Aggregate(tries, minSuccess) | ||
| } | ||
|
|
||
| func probeTCP(wc workerClients, n *discv4.Node, timeout time.Duration, attempts, minSuccess int) probe.Summary { | ||
| if wc.tcpInit != nil { | ||
| return probe.LocalSummary(wc.tcpInit) | ||
| } | ||
| perProbe := timeout / time.Duration(attempts) | ||
| if perProbe < time.Second { | ||
| perProbe = time.Second | ||
| } | ||
|
|
||
| tries := make([]probe.Attempt, 0, attempts) | ||
| for i := 0; i < attempts; i++ { | ||
| if i > 0 { | ||
| time.Sleep(150 * time.Millisecond) | ||
| } | ||
| out := wc.tcp.Dial(n, perProbe) | ||
| switch out.Outcome { | ||
| case rlpx.OutcomeOK: | ||
| tries = append(tries, probe.Attempt{ | ||
| Outcome: probe.OutcomeOK, | ||
| RTT: time.Duration(out.RTT) * time.Millisecond, | ||
| }) | ||
| case rlpx.OutcomeWarning: | ||
| tries = append(tries, probe.Attempt{ | ||
| Outcome: probe.OutcomeWarning, | ||
| Message: out.Message, | ||
| }) | ||
| default: | ||
| tries = append(tries, probe.Attempt{ | ||
| Outcome: probe.OutcomeFail, | ||
| Message: out.Message, | ||
| }) | ||
| } | ||
| } | ||
| return probe.Aggregate(tries, minSuccess) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Honor the configured timeout as an actual probe budget.
probeUDP never uses timeout at all, and probeTCP can expand a smaller timeout to at least attempts * 1s plus backoff because the 1-second floor is applied per attempt. That means the checker timeout does not actually bound per-node probe duration.
🤖 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 `@backend/internal/service/bootnode_checker.go` around lines 205 - 258, The
probe helpers are not respecting the configured timeout as a real per-node
budget: probeUDP ignores timeout entirely, and probeTCP can exceed it because
the minimum 1-second floor is applied per attempt. Update probeUDP and probeTCP
in bootnode_checker.go so the timeout is divided across attempts and enforced as
a total budget, with backoff/counting included, while keeping the existing
attempt aggregation via probe.Aggregate and the local-init fast paths in
workerClients.
Comment on lines
+37
to
+48
| func LoadBootnodes(cfg *config.Config) ([]*discv4.Node, error) { | ||
| if !cfg.EnableBootnodeHealth { | ||
| return nil, fmt.Errorf("bootnode health disabled") | ||
| } | ||
| if strings.EqualFold(strings.TrimSpace(cfg.BootnodeNetwork), "devnet") { | ||
| return loadDevnetBootnodes() | ||
| } | ||
| url, err := BootnodesURLForNetwork(cfg.BootnodeNetwork) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return discv4.LoadNodesFromURL(url) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Avoid coupling every health pass to a fresh GitHub fetch.
backend/internal/service/bootnode_checker.go:111-134 calls this loader on every check cycle, and LoadBootnodes always falls through to discv4.LoadNodesFromURL. That means a transient GitHub/raw failure or rate limit aborts the whole probe round before any existing node is checked. Cache the parsed list and refresh it on startup or a long TTL instead of re-downloading it every interval.
🤖 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 `@backend/internal/service/bootnode_load.go` around lines 37 - 48,
`LoadBootnodes` is re-fetching bootnode data on every health check, which ties
each probe cycle to `discv4.LoadNodesFromURL` and makes transient network/GitHub
failures abort checks. Update `LoadBootnodes` and the caller in
`bootnode_checker` to use a cached parsed bootnode list, refreshing it on
startup or behind a long TTL instead of downloading on every interval. Keep the
network-specific selection logic (`BootnodesURLForNetwork`,
`loadDevnetBootnodes`) but decouple it from per-check fetching.
Comment on lines
+162
to
+172
| const { | ||
| report: bootnodeReport, | ||
| loading: bootnodeLoading, | ||
| checking: bootnodeChecking, | ||
| error: bootnodeError, | ||
| disabled: bootnodeDisabled, | ||
| triggerCheck: triggerBootnodeCheck, | ||
| } = useBootnodePolling({ | ||
| apiUrl: API_URL, | ||
| enabled: Boolean(API_URL), | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don't disable bootnode polling for the same-origin default.
API_URL already falls back to '' elsewhere in this file, so this gate prevents /v2/bootnodes/* polling in the default same-origin setup while the panel still renders.
Suggested fix
} = useBootnodePolling({
apiUrl: API_URL,
- enabled: Boolean(API_URL),
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const { | |
| report: bootnodeReport, | |
| loading: bootnodeLoading, | |
| checking: bootnodeChecking, | |
| error: bootnodeError, | |
| disabled: bootnodeDisabled, | |
| triggerCheck: triggerBootnodeCheck, | |
| } = useBootnodePolling({ | |
| apiUrl: API_URL, | |
| enabled: Boolean(API_URL), | |
| }); | |
| const { | |
| report: bootnodeReport, | |
| loading: bootnodeLoading, | |
| checking: bootnodeChecking, | |
| error: bootnodeError, | |
| disabled: bootnodeDisabled, | |
| triggerCheck: triggerBootnodeCheck, | |
| } = useBootnodePolling({ | |
| apiUrl: API_URL, | |
| }); |
🤖 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 `@frontend/src/App.tsx` around lines 162 - 172, Bootnode polling is being
disabled by the Boolean(API_URL) gate in App’s useBootnodePolling setup, which
blocks the default same-origin case where API_URL falls back to ''. Update the
App component so useBootnodePolling still runs when API_URL is
empty/same-origin, and only disable it for truly invalid or intentionally off
configurations. Keep the existing API_URL handling consistent with the rest of
the file and preserve the polling flow for /v2/bootnodes/* in the default setup.
Comment on lines
+331
to
+344
| <button | ||
| type="button" | ||
| onClick={copy} | ||
| className="text-xs px-3 py-1 rounded-full focus:outline-none" | ||
| style={{ background: '#f0f4f8', color: '#6b7280', border: 'none', cursor: 'pointer' }} | ||
| > | ||
| Copy | ||
| </button> | ||
| <button | ||
| type="button" | ||
| onClick={onClose} | ||
| className="text-xs px-3 py-1 rounded-full focus:outline-none" | ||
| style={{ background: '#242c6d', color: '#fff', border: 'none', cursor: 'pointer' }} | ||
| > |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restore a visible focus style on the new actions.
These controls remove the native outline with focus:outline-none but never add a replacement, so keyboard users lose track of the active button.
Suggested fix
- className="text-xs px-3 py-1 rounded-full transition-colors focus:outline-none"
+ className="text-xs px-3 py-1 rounded-full transition-colors focus:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:ring-blue-600"Apply the same pattern to the modal, error, collapse, and pagination buttons in this component.
Also applies to: 399-414, 631-661, 720-749
🤖 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 `@frontend/src/components/BootnodesPanel.tsx` around lines 331 - 344, The new
action buttons in BootnodesPanel remove the native focus outline with
focus:outline-none but do not provide a replacement, so keyboard focus becomes
invisible. Update the affected button styles in the copy/close controls and the
other button groups in this component (the modal, error, collapse, and
pagination actions) to use a visible focus state, such as a focus ring or
outlined border, consistent with the existing button styling.
Comment on lines
+759
to
+769
| {BOOTNODE_COLUMNS.map((col) => ( | ||
| <th | ||
| key={col.key} | ||
| className={`px-3 py-2 text-${col.align}${col.sortable ? ' cursor-pointer' : ''}`} | ||
| onClick={col.sortable ? () => handleSort(col.key) : undefined} | ||
| onMouseEnter={(e) => showHeaderTooltip(e, col.tooltip)} | ||
| onMouseLeave={hideTooltip} | ||
| onMouseMove={moveTooltip} | ||
| > | ||
| {col.label} | ||
| {col.sortable && <SortIcon colKey={col.key} />} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make the sortable headers keyboard-operable.
The new sort controls live on <th> click handlers, which are not focusable and cannot be activated from the keyboard. Put a real button inside the header cell and expose the current sort via aria-sort.
Suggested fix
- <th
- key={col.key}
- className={`px-3 py-2 text-${col.align}${col.sortable ? ' cursor-pointer' : ''}`}
- onClick={col.sortable ? () => handleSort(col.key) : undefined}
- onMouseEnter={(e) => showHeaderTooltip(e, col.tooltip)}
- onMouseLeave={hideTooltip}
- onMouseMove={moveTooltip}
- >
- {col.label}
- {col.sortable && <SortIcon colKey={col.key} />}
- </th>
+ <th
+ key={col.key}
+ scope="col"
+ aria-sort={
+ activeSort.key === col.key
+ ? activeSort.dir === 'asc' ? 'ascending' : 'descending'
+ : 'none'
+ }
+ className={col.align === 'right' ? 'px-3 py-2 text-right' : 'px-3 py-2 text-left'}
+ onMouseEnter={(e) => showHeaderTooltip(e, col.tooltip)}
+ onMouseLeave={hideTooltip}
+ onMouseMove={moveTooltip}
+ >
+ <button
+ type="button"
+ onClick={() => handleSort(col.key)}
+ className="w-full text-inherit focus:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:ring-blue-600"
+ >
+ {col.label}
+ {col.sortable && <SortIcon colKey={col.key} />}
+ </button>
+ </th>🤖 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 `@frontend/src/components/BootnodesPanel.tsx` around lines 759 - 769, The
sortable table headers in BootnodesPanel are only clickable on the <th>, which
makes them unreachable by keyboard. Move the sorting action into a वास्तविक
button inside the header cell in the BOOTNODE_COLUMNS render loop, and keep the
tooltip behavior on the cell if needed. Make the button focusable/activatable by
keyboard and expose the current sort state with aria-sort on the corresponding
header based on the active sort key and direction. Ensure handleSort and
SortIcon still drive the same sorting logic and visual state.
benjamin202410
approved these changes
Jun 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
docker-compose.yml.start-wizard.shto export bootnode network configuration.main.goto initialize bootnode health checking if enabled.Summary by CodeRabbit
New Features
Bug Fixes