Skip to content

feat: RPC node failover system + accessibility fixes for modals#285

Closed
blazeapps007 wants to merge 19 commits into
steemit:nextfrom
blazeapps007:next
Closed

feat: RPC node failover system + accessibility fixes for modals#285
blazeapps007 wants to merge 19 commits into
steemit:nextfrom
blazeapps007:next

Conversation

@blazeapps007
Copy link
Copy Markdown
Contributor

This PR introduces two focused improvements:

1. Configurable Steem RPC Node Failover

  • Add six predefined Steem RPC nodes to .env.example with comma-separated failover support: api.steem.fans, api2.justyy.com, api.steemit.com, api.moecki.online, steem.senior.workers.dev, steemd.steemworld.org
  • Refactor withFailover in server.ts to use AsyncLocalStorage for per-request preferred node selection via X-Steem-RPC header, preserving method signatures and using remaining nodes as fallback
  • Export withRpcOverride and applyRpcOverride helper (src/lib/api/with-rpc-override.ts) to validate header against NEXT_PUBLIC_RPC_NODES before applying override
  • Update all twelve query API routes to wrap SteemService calls with applyRpcOverride for per-request node honouring
  • Add client-side RPC node store (src/lib/rpc-node.ts) using module-level + useSyncExternalStore pattern (matching theme store); persists selection to localStorage and syncs across tabs
  • Update apiClient to send X-Steem-RPC header on all query fetches
  • Add RPC Node selector UI in SidePanel (visible when more than one node configured); displays hostname labels and highlights active node

2. Accessibility Fixes for Modals and Navigation Drawer

  • Add sr-only DialogDescription to transfer, power-down, and delegate modal branches that previously had a title but no description, resolving Radix UI warnings about missing accessible descriptions
  • Add aria-describedby={undefined} to SheetContent in SidePanel since it functions as a navigation drawer and intentionally has no description

Testing Notes

  • Verify RPC node selection persists across page reloads and syncs between tabs
  • Confirm X-Steem-RPC header is sent on query requests and respected server-side
  • Check that Radix UI warnings no longer appear in console for modals
  • Ensure SidePanel navigation drawer renders without accessibility warnings

blazeapps007 and others added 19 commits April 25, 2026 14:36
Replace useSyncExternalStore with useState + useEffect in Header so the
remembered username is always null on the initial render (matching the
server), then updated after hydration. Apply the same deferred pattern
to useTheme, which was reading localStorage inside the useState
initializer — a value the server can never produce, causing React to
render different trees on server vs client.
Cross-tab sync — window.addEventListener('storage', ...) in the module init block. When another tab writes to THEME_KEY, _theme is updated and _notify() is called so all subscribers in this tab re-render.

Duplicated classList ops — both sites kept, with comments explaining why: changeTheme applies synchronously for FOUC prevention, useLayoutEffect is the safety net for mount and cross-tab updates that come through _notify().

Set mutation during iteration — all forEach calls now go through _notify(), which snapshots with Array.from(_listeners) before iterating.

cycleTheme stability — removed the theme dependency by reading _theme directly from the module store. cycleTheme is now stable (only depends on changeTheme, which is itself stable with []), so it won't invalidate memoized children if one is added later.
Replace the single shared store with a makeStore factory and create the store in Providers via a persistent useRef so each server request gets an isolated store (prevents auth state leaking) while the client keeps a single instance. Update store types to AppStore/RootState/AppDispatch. Clarify Header's useSyncExternalStore usage in a comment for SSR-safe avatar initialization. Make getCurrentTheme SSR-safe by returning the internal _theme instead of reading localStorage.
Lock in the SSR-isolation invariant introduced in this PR so a future
refactor cannot quietly revert makeStore back to a module-level singleton
without failing CI.
The repo's vitest config requires 80% line/statement coverage, but ui.ts
and wallet.ts had no tests, dragging the global stats to 79.54% lines /
77.30% statements and failing CI. Add reducer tests for every action in
both slices.

After this change:
- Lines:      79.54% -> 84.46%
- Statements: 77.30% -> 81.91%
Introduce AUTH_KEY_FIELDS and AuthKeyField type and update serializableCheck to ignore auth key paths and the auth/setCredentials action to avoid middleware warnings for private keys. Disable Redux DevTools in production entirely, and in development add stateSanitizer and actionSanitizer to replace live private-key fields with "[REDACTED]" in the auth slice and in setCredentials actions so keys are never shown in plain text to extensions or logs.
Introduce AUTH_KEY_FIELDS constant covering all five key fields in the
auth slice (ownerKey, activeKey, postingKey, memoKey, privateKey) and
apply two layers of protection:

Production:
  devTools: false — the Redux DevTools extension cannot inspect the
  store on a real user session under any circumstances.

Development:
  stateSanitizer replaces every non-null key field with '[REDACTED]'
  before the extension renders state, so a developer's own keys are
  never shown in plain text in the DevTools panel or time-travel log.

  actionSanitizer applies the same redaction to auth/setCredentials
  payloads so keys are not captured at the moment of login either.

Also fixes the pre-existing serializableCheck config which referenced
a non-existent action ('auth/setPrivateKey') and only listed one of
the five key paths — updated to 'auth/setCredentials' and all paths.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
```
feat: add configurable Steem RPC node failover with per-request override

- Add six predefined Steem RPC nodes to .env.example with comma-separated failover support: api.steem.fans, api2.justyy.com, api.steemit.com, api.moecki.online, steem.senior.workers.dev, steemd.steemworld.org
- Refactor withFailover in server.ts to use AsyncLocalStorage for per-request preferred node selection via X-Steem-RPC header, preserving method signatures and using remaining nodes as fallback
- Export withRpcOverride and applyRpcOverride helper (src/lib/api/with-rpc-override.ts) to validate header against NEXT_PUBLIC_RPC_NODES before applying override
- Update all twelve query API routes to wrap SteemService calls with applyRpcOverride for per-request node honouring
- Add client-side RPC node store (src/lib/rpc-node.ts) using module-level + useSyncExternalStore pattern matching theme store; persists selection to localStorage and syncs across tabs
- Update apiClient to send X-Steem-RPC header on all query fetches
- Add RPC Node selector UI in SidePanel (visible when more than one node configured); displays hostname labels and highlights active node
```
Add sr-only DialogDescription to transfer, power-down, and delegate
modal branches that had a title but no description, causing Radix UI
to warn about missing accessible descriptions. Add aria-describedby
={undefined} to SheetContent in SidePanel since it is a navigation
drawer and intentionally has no description.
Broadcast calls now compose withCSRFHeader(withRpcHeader(...)) so the
selected RPC node is forwarded to broadcast endpoints alongside CSRF.
fetchTransactionHeader also gains the RPC header for consistency.
Auth-only endpoints (login, logout, challenge) remain CSRF-only since
they do not communicate with Steem nodes.

Update steem-client.test.ts expectations to match: broadcast assertions
include X-Steem-RPC, GET query assertions include X-Steem-RPC, and auth
assertions remain unchanged. Adds DEFAULT_RPC_NODE constant to avoid
hardcoding the URL in multiple places.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extract devStateSanitizer and devActionSanitizer from store/index.ts into
named exports so the key-redaction logic is directly testable without
requiring the Redux DevTools browser extension.

Add tests/unit/rpc-node.test.tsx covering: default node selection,
localStorage initialisation on module load, setNode persistence and
validation, cross-tab storage event reaction, and hook unmount cleanup.

Add devStateSanitizer / devActionSanitizer suites to store.test.ts
covering key redaction, null-field skip, pass-through of unrelated
actions, and preservation of non-key fields.

Raises statement coverage from 79.12% to 82.12% and function coverage
from 79.22% to 82.75%, clearing the 80% threshold required by CI.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@ety001
Copy link
Copy Markdown
Member

ety001 commented May 21, 2026

Review Comment: RPC Node Selector Feature — Recommendation: Defer to Post-Stabilization Phase

After careful review, I recommend not merging this feature at the current stage of the refactoring. Here's the architectural rationale:

The core design principle of this refactored version is that the browser should never communicate directly with Steem RPC nodes. Instead, the Next.js API layer acts as the sole gateway: the browser only signs transactions locally with the user's private keys, and all network communication (queries, broadcasts, health checks) is proxied through the server-side API routes.

This PR introduces a user-selectable RPC node switcher, which implies a mental model where users "choose which node their browser talks to." However, in the current architecture, the browser does not talk to any node directly — it only talks to . The header is effectively just a "preference hint" passed to the server, and this introduces concurrency risks, cache isolation issues, and inconsistencies between query and broadcast paths without clear user-visible benefits.

Technical Issues Identified

  1. Concurrency race condition: mutates a global singleton. Under concurrent requests, one request may overwrite the RPC URL while another is suspended mid-flight.
  2. Broadcast routes ignore the header: All routes do not call , so user node selection is silently ignored for writes.
  3. Cache cross-contamination: keys do not include the RPC node URL. A cache entry from node A can be served to a user who explicitly selected node B.
  4. ** lacks failover**: Login signature verification calls without , so a single node outage blocks logins.
  5. Environment variable mismatch risk: (from ) and (from ) can diverge, leading to silent fallback behavior.

Recommendation

  • Do not merge PR285 for the initial stable release. The default server-side failover mechanism ( with left-to-right fallback) is sufficient for launch.
  • Revisit this feature after the refactored version is live and stable. Once the new architecture has been running in production and the server-side RPC layer is battle-tested, we can properly explore user-defined node preferences — ideally implemented as a server-side configuration (e.g., per-session or per-user server setting) rather than a client-side selector, ensuring the browser still never holds RPC topology knowledge.

The code quality in this PR is otherwise good (tests, a11y fixes, AsyncLocalStorage usage), but the feature itself is architecturally premature for the current phase.

@ety001
Copy link
Copy Markdown
Member

ety001 commented May 21, 2026

Closing per review discussion. The feature is architecturally premature for the current refactoring phase. We will revisit user-selectable RPC nodes after the refactored version is stable in production.

@ety001 ety001 closed this May 21, 2026
@blazeapps007
Copy link
Copy Markdown
Contributor Author

blazeapps007 commented May 21, 2026

@ety001 So everything will go through Next.js API layer ?

@ety001
Copy link
Copy Markdown
Member

ety001 commented May 21, 2026

@ety001 So everything will go through Next.js API layer ?

Yes. All user traffic will be proxied through the API layer implemented in Next.js.

The purpose is to better distinguish between genuine website traffic and developer traffic at the api.steemit.com endpoint. When api.steemit.com experiences significant traffic fluctuations, we can use the OpenResty layer in front of Jussi to ensure website traffic gets through while rate-limiting or even cutting off other traffic.

At the same time, we can also set up Cloudflare JS Challenge for the entire site to protect the Next.js-implemented API from being abused by bots.

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