Conversation
📝 WalkthroughWalkthroughProject renamed and versioned to 4.0.0; major dependency and Node engine upgrades. Large FAQ and utilities (blockchain, safeTransactions) removed. Login/auth flow now calls isNodeStarted and no longer fetches channelsCorrupted; multiple UI prop destructurings and data-field renames applied. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 1
🤖 Fix all issues with AI agents
In `@src/store/slices/auth/actionsAsync.ts`:
- Around line 1-8: The code currently overwrites the isNodeStarted response with
getInfo so the node-started signal is ignored; modify the async flow that calls
isNodeStarted() and getInfo() (e.g., in the login async action) to store the
isNodeStarted result in its own variable (e.g., isStartedResponse) and validate
it (check boolean or a started/isStarted field per SDK) before proceeding to
call getInfo() or completing login; if the node is not started, abort the login
flow and dispatch the appropriate error/failed action instead of proceeding.
🧹 Nitpick comments (2)
src/pages/node/info/index.tsx (1)
416-445: Remove commented-out code instead of leaving it in place.Commented-out JSX blocks create dead code that clutters the codebase and causes confusion about whether the feature is temporarily disabled or permanently removed. If this UI is being removed as part of the v4 migration, delete the code entirely—version control preserves the history if it's needed later.
Additionally, the variables
indexerLastLogBlock(line 65) andindexerLastLogChecksum(line 66) are now unused since they were only referenced by this commented-out section.♻️ Suggested cleanup
Remove lines 65-66:
- const indexerLastLogBlock = useAppSelector((store) => store.node.info.data?.indexerLastLogBlock); // >=2.2.0 - const indexerLastLogChecksum = useAppSelector((store) => store.node.info.data?.indexerLastLogChecksum); // >=2.2.0And delete the entire commented block (lines 416-445) rather than leaving it commented.
src/store/slices/auth/actionsAsync.ts (1)
90-92: Use a safe formatter for FETCH_ERROR details.
JSON.stringify(e)can throw on circular structures and often yields{}forErrorobjects. Consider a safe formatter to avoid losing context or breaking the catch path.🔧 Safer error formatting
- return rejectWithValue({ - data: 'Unable to connect to the node.\n\nError:\n' + JSON.stringify(e, null, 2), - type: 'FETCH_ERROR', - }); + const errorText = + e instanceof Error + ? `${e.name}: ${e.message}` + : (() => { + try { + return JSON.stringify(e, null, 2); + } catch { + return String(e); + } + })(); + return rejectWithValue({ + data: 'Unable to connect to the node.\n\nError:\n' + errorText, + type: 'FETCH_ERROR', + });
| import { | ||
| GetInfoResponseType, | ||
| GetAddressesResponseType, | ||
| GetBalancesResponseType, | ||
| IsNodeStartedResponseType, | ||
| api, | ||
| utils | ||
| } from '@hoprnet/hopr-sdk'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the full file to understand the context around the imports and Promise.allSettled
cat -n src/store/slices/auth/actionsAsync.tsRepository: hoprnet/node-admin-ui
Length of output: 5212
🌐 Web query:
@hoprnet/hopr-sdk IsNodeStartedResponseType API documentation
💡 Result:
Do you want a link to the IsNodeStartedResponseType definition (source / package docs) or a copy of the TypeScript type/fields? I can search the @hoprnet/hopr-sdk package and return the exact API/type.
🏁 Script executed:
# Search for node_modules or any SDK type definitions in the repo
fd -t f "hopr-sdk" --max-one-resultRepository: hoprnet/node-admin-ui
Length of output: 298
🌐 Web query:
@hoprnet/hopr-sdk npm package IsNodeStartedResponseType definition
💡 Result:
I couldn't find a published package or source for @hoprnet/hopr-sdk or an exported type named IsNodeStartedResponseType. Could you either:
- paste the file or code that references IsNodeStartedResponseType, or
- share a link (GitHub / npm / repo) to the package/version you mean?
With that I can extract the exact TypeScript definition.
🏁 Script executed:
# Also check if there's package.json to see the SDK version
cd . && find . -name "package.json" -type f | head -5 | xargs grep -l "hopr-sdk" 2>/dev/null || echo "No package.json found"Repository: hoprnet/node-admin-ui
Length of output: 79
🏁 Script executed:
cat package.json | grep -A2 -B2 "hopr-sdk"Repository: hoprnet/node-admin-ui
Length of output: 266
🏁 Script executed:
# Check if there's a GitHub repo reference or documentation
cd . && find . -name "tsconfig.json" -o -name ".npmrc" -o -name ".yarnrc" | head -3Repository: hoprnet/node-admin-ui
Length of output: 99
🏁 Script executed:
# Let's also check the node_modules for the SDK
find node_modules/@hoprnet -name "*.d.ts" -type f 2>/dev/null | grep -i "isnode\|started" | head -10 || echo "SDK types not directly found; checking index"Repository: hoprnet/node-admin-ui
Length of output: 47
🌐 Web query:
hoprnet node-admin-ui GitHub repository isNodeStarted API
💡 Result:
The admin-ui doesn't implement its own isNodeStarted endpoint — it reads the node state from the HOPRd REST API. See the admin-ui repo (Node Admin connects to a hoprd API endpoint) and the HOPR docs which state hoprd exposes an OpenAPI spec at /api-docs/openapi.json (inspect that OpenAPI to find the exact field/path that reports "node started"). [1][2]
Sources:
- hoprnet/admin-ui (repo README / connect-to-node instructions). [1]
- HOPR docs: Admin UI / hoprd REST API (openapi.json available at /api-docs/openapi.json). [2]
🏁 Script executed:
# Find the hopr-sdk package directory
find node_modules/@hoprnet -type f -name "*.ts" -o -name "*.d.ts" 2>/dev/null | head -20Repository: hoprnet/node-admin-ui
Length of output: 47
🏁 Script executed:
# Try to find the index or types file for hopr-sdk
ls -la node_modules/@hoprnet/hopr-sdk/ 2>/dev/null | head -20Repository: hoprnet/node-admin-ui
Length of output: 47
🏁 Script executed:
# Check if there are type definitions that show IsNodeStartedResponseType
grep -r "IsNodeStartedResponseType" node_modules/@hoprnet/hopr-sdk 2>/dev/null || echo "Type not found"Repository: hoprnet/node-admin-ui
Length of output: 79
🏁 Script executed:
# Search for any other references to nodeStarted or the isNodeStarted result
rg "nodeStarted|isNodeStarted" --type ts --type tsx -nRepository: hoprnet/node-admin-ui
Length of output: 92
🏁 Script executed:
# Also check if there's any validation logic that might use isNodeStarted in the thunk
rg "started" src/store/slices/auth/actionsAsync.ts -i -nRepository: hoprnet/node-admin-ui
Length of output: 286
Store isNodeStarted result separately and validate node is started before login.
Lines 50–51 overwrite the isNodeStarted response with getInfo, so the node-started signal is completely ignored. If the SDK returns a "not started" flag or object field, login will succeed anyway. Either gate login on the started state (check the returned value for a boolean or started/isStarted field per SDK docs), or remove the call if it's only for side effects.
Suggested fix
- let info, addresses, balances;
+ let info, addresses, balances, nodeStarted;
- if (calls[0].status === 'fulfilled') info = calls[0].value as IsNodeStartedResponseType;
- if (calls[1].status === 'fulfilled') info = calls[1].value as GetInfoResponseType;
+ if (calls[0].status === 'fulfilled') nodeStarted = calls[0].value as IsNodeStartedResponseType;
+ if (calls[1].status === 'fulfilled') info = calls[1].value as GetInfoResponseType;
if (calls[0].status === 'rejected') throw new sdkApiError(calls[0].reason);
if (calls[1].status === 'rejected') throw new sdkApiError(calls[1].reason);
if (calls[2].status === 'rejected') throw new sdkApiError(calls[2].reason);
if (calls[3].status === 'rejected') throw new sdkApiError(calls[3].reason);
+ const isStarted =
+ typeof nodeStarted === 'boolean'
+ ? nodeStarted
+ : (nodeStarted as { started?: boolean; isStarted?: boolean } | undefined)?.started ??
+ (nodeStarted as { started?: boolean; isStarted?: boolean } | undefined)?.isStarted;
+ if (isStarted === false) {
+ return rejectWithValue({
+ data: 'Node has not started yet. Please wait and retry.',
+ type: 'CUSTOM_ERROR',
+ });
+ }🤖 Prompt for AI Agents
In `@src/store/slices/auth/actionsAsync.ts` around lines 1 - 8, The code currently
overwrites the isNodeStarted response with getInfo so the node-started signal is
ignored; modify the async flow that calls isNodeStarted() and getInfo() (e.g.,
in the login async action) to store the isNodeStarted result in its own variable
(e.g., isStartedResponse) and validate it (check boolean or a started/isStarted
field per SDK) before proceeding to call getInfo() or completing login; if the
node is not started, abort the login flow and dispatch the appropriate
error/failed action instead of proceeding.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/pages/node/tickets.tsx`:
- Around line 202-203: The JSX in the tickets page contains an invalid
JavaScript comment inside an attribute (the commented `title={`...`}` line)
which breaks compilation; open the JSX element in src/pages/node/tickets.tsx
where the `title={`Minimum allowed winning probability...`}` attribute is
defined and either remove the dead `// title={...}` line entirely or replace it
with a valid JSX comment syntax (/* ... */) or a proper conditional/rendered
string so there are no `//` comments inside JSX attributes.
♻️ Duplicate comments (1)
src/store/slices/auth/actionsAsync.ts (1)
30-62: Gate login onisNodeStarted(result currently unused).
nodeStartedis assigned but never checked, so a “not started” response that doesn’t throw still allows login. Either validate the response before returninginfo, or remove the call if it’s only for side effects.✅ Suggested fix (adjust to actual SDK response shape)
- if (calls[0].status === 'fulfilled') nodeStarted = calls[0].value as IsNodeStartedResponseType; + if (calls[0].status === 'fulfilled') nodeStarted = calls[0].value as IsNodeStartedResponseType; if (calls[1].status === 'fulfilled') info = calls[1].value as GetInfoResponseType; @@ - return info; + const isStarted = + typeof nodeStarted === 'boolean' + ? nodeStarted + : (nodeStarted as { started?: boolean; isStarted?: boolean } | undefined)?.started ?? + (nodeStarted as { started?: boolean; isStarted?: boolean } | undefined)?.isStarted; + if (isStarted === false) { + return rejectWithValue({ + data: 'Your node has not started yet. Please wait and retry.', + type: 'CUSTOM_ERROR', + }); + } + return info;`@hoprnet/hopr-sdk` IsNodeStartedResponseType response fields and semantics (version 4.0.0-pr.159-20260121135601)
🧹 Nitpick comments (2)
src/pages/node/tickets.tsx (1)
29-29: Remove unusedinfovariable declaration.The
infovariable is no longer used in this file. It was previously referenced viainfo?.networkin the tooltip on line 202, but that usage has been replaced with a static string. Remove the unused selector declaration from line 29.src/store/slices/node/actionsAsync.ts (1)
464-480: Enforce non‑optional response contracts.These thunks now claim non‑optional response types, but they don’t guard against
undefinedresults from the SDK. If the SDK can returnundefined(even rarely), callers will get a fulfilled action with an invalid payload. Consider adding a runtime check or confirm the SDK’s contract guarantees a defined response.✅ Optional guard to uphold the non‑optional contract
const res = await closeChannel(payload); + if (!res) { + return rejectWithValue({ status: 'Empty closeChannel response' }); + } dispatch(getChannelsThunk(payload)); return res;const res = await openChannel(payload); + if (!res) { + return rejectWithValue({ status: 'Empty openChannel response' }); + } return res;const res = await fundChannel(payload); + if (!res) { + return rejectWithValue({ status: 'Empty fundChannel response' }); + } return res;Also applies to: 498-510, 514-526
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)
src/pages/node/aliases.tsx (1)
63-76: LGTM! The logic correctly handles missing peers and formats dates appropriately. The code follows the same pattern aspeers.tsxand uses thelastUpdatefield consistently.Minor note: The
nodeAddress ?check on line 63 is redundant sincenodeAddressalways comes fromObject.keys()and is always a string; the optional chaining?.lastUpdatealready handles undefined cases.
🤖 Fix all issues with AI agents
In `@src/store/slices/auth/actionsAsync.ts`:
- Around line 85-88: Update the user-facing error string where the auth async
action builds the error payload: change the phrase "seems to low" to "seems too
low" in the object with keys data and type (the message that includes
addresses?.native and type: 'CUSTOM_ERROR') so the message reads "Your xDai
balance seems too low to operate the node."
In `@src/store/slices/node/actionsAsync.ts`:
- Around line 494-507: Add a condition block to openChannelThunk that inspects
the current state (RootState) and cancels duplicate dispatches when a channel to
the same peer is already being opened: in createAsyncThunk('node/openChannel',
...) add a condition function that reads state.node.outgoingOpening and returns
false if outgoingOpening[payload.destination] (or the appropriate
payload.peerAddress field) is truthy, mirroring the guard used in
closeChannelThunk, so repeated dispatches for the same destination are
prevented.
♻️ Duplicate comments (1)
src/store/slices/auth/actionsAsync.ts (1)
30-62: ValidateisNodeStartedbefore completing login.
isNodeStartedis fetched but never used to gate the login flow, so users can proceed while the node is still starting. Consider rejecting with a clear error when the SDK reports “not started” (field name depends on SDK).✅ Suggested fix (shape may vary by SDK)
- let info, addresses, balances, nodeStarted; + let info, addresses, balances, nodeStarted; try { const calls = await Promise.allSettled([ isNodeStarted({ apiEndpoint: apiEndpoint, apiToken: apiToken, }), @@ if (calls[0].status === 'rejected') throw new sdkApiError(calls[0].reason); @@ + const isStarted = + typeof nodeStarted === 'boolean' + ? nodeStarted + : (nodeStarted as { started?: boolean; isStarted?: boolean } | undefined)?.started ?? + (nodeStarted as { started?: boolean; isStarted?: boolean } | undefined)?.isStarted; + if (isStarted === false) { + return rejectWithValue({ + data: 'Your Node seems to be starting, wait a couple of minutes before accessing it.', + type: 'CUSTOM_ERROR', + }); + } + return info;Please confirm the actual response shape for
IsNodeStartedResponseTypein@hoprnet/hopr-sdk@4.0.0-pr.159-20260121135601.IsNodeStartedResponseType fields in `@hoprnet/hopr-sdk` 4.0.0-pr.159-20260121135601 (isNodeStarted response)
🧹 Nitpick comments (1)
src/pages/node/configuration.tsx (1)
91-108: Unusedconfigsarray.The
configsarray is constructed but never consumed. This appears to be leftover from previous logic (possibly related to the unusedcalculateTicketsandupdateStrategyStringfunctions). Consider removing it if no longer needed.♻️ Suggested removal
try { - const configs: StrategyConfig[] = [ - { - path: ['AutoFunding', 'min_stake_threshold'], - value: strategy.strategies?.AutoFunding?.min_stake_threshold?.replace(' wxHOPR', ''), - }, - { - path: ['AutoFunding', 'funding_amount'], - value: strategy.strategies?.AutoFunding?.funding_amount?.replace(' wxHOPR', ''), - }, - { - path: ['AutoRedeeming', 'minimum_redeem_ticket_value'], - value: strategy.strategies?.AutoRedeeming?.minimum_redeem_ticket_value?.replace(' wxHOPR', ''), - }, - { - path: ['AutoRedeeming', 'on_close_redeem_single_tickets_value_min'], - value: strategy.strategies?.AutoRedeeming?.on_close_redeem_single_tickets_value_min?.replace(' wxHOPR', ''), - }, - ]; - let strategiesString = yaml.dump(strategyTMP);
| data: | ||
| 'Unable to connect.\n\n' + | ||
| `Your xDai balance seems to low to operate the node.\nPlease top up your node.\nAddress: ${addresses?.native}`, | ||
| type: 'NOT_ELIGIBLE_ERROR', | ||
| type: 'CUSTOM_ERROR', |
There was a problem hiding this comment.
Fix typo in user-facing error message.
Minor grammar issue: “seems to low” → “seems too low”.
✏️ Suggested edit
- `Your xDai balance seems to low to operate the node.\nPlease top up your node.\nAddress: ${addresses?.native}`,
+ `Your xDai balance seems too low to operate the node.\nPlease top up your node.\nAddress: ${addresses?.native}`,📝 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.
| data: | |
| 'Unable to connect.\n\n' + | |
| `Your xDai balance seems to low to operate the node.\nPlease top up your node.\nAddress: ${addresses?.native}`, | |
| type: 'NOT_ELIGIBLE_ERROR', | |
| type: 'CUSTOM_ERROR', | |
| data: | |
| 'Unable to connect.\n\n' + | |
| `Your xDai balance seems too low to operate the node.\nPlease top up your node.\nAddress: ${addresses?.native}`, | |
| type: 'CUSTOM_ERROR', |
🤖 Prompt for AI Agents
In `@src/store/slices/auth/actionsAsync.ts` around lines 85 - 88, Update the
user-facing error string where the auth async action builds the error payload:
change the phrase "seems to low" to "seems too low" in the object with keys data
and type (the message that includes addresses?.native and type: 'CUSTOM_ERROR')
so the message reads "Your xDai balance seems too low to operate the node."
| const openChannelThunk = createAsyncThunk<OpenChannelResponseType, OpenChannelPayloadType, { state: RootState }>( | ||
| 'node/openChannel', | ||
| async (payload, { rejectWithValue }) => { | ||
| try { | ||
| const res = await openChannel(payload); | ||
| return res; | ||
| } catch (e) { | ||
| if (e instanceof sdkApiError) { | ||
| return rejectWithValue(e); | ||
| } | ||
| return rejectWithValue({ status: JSON.stringify(e) }); | ||
| } | ||
| return rejectWithValue({ status: JSON.stringify(e) }); | ||
| } | ||
| }); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n -A5 'openChannelThunk' --type=ts -g '!**/actionsAsync.ts'Repository: hoprnet/node-admin-ui
Length of output: 1215
🏁 Script executed:
wc -l src/components/Modal/node/OpenChannelModal.tsxRepository: hoprnet/node-admin-ui
Length of output: 115
🏁 Script executed:
wc -l src/components/Modal/node/OpenMultipleChannelsModal.tsxRepository: hoprnet/node-admin-ui
Length of output: 124
🏁 Script executed:
sed -n '70,110p' src/components/Modal/node/OpenChannelModal.tsxRepository: hoprnet/node-admin-ui
Length of output: 1636
🏁 Script executed:
sed -n '40,75p' src/components/Modal/node/OpenMultipleChannelsModal.tsxRepository: hoprnet/node-admin-ui
Length of output: 1314
🏁 Script executed:
rg -n 'outgoingOpening' --type=tsRepository: hoprnet/node-admin-ui
Length of output: 814
🏁 Script executed:
sed -n '1,90p' src/components/Modal/node/OpenChannelModal.tsx | grep -n -A2 -B2 'disabled\|Button'Repository: hoprnet/node-admin-ui
Length of output: 889
🏁 Script executed:
sed -n '1,90p' src/components/Modal/node/OpenMultipleChannelsModal.tsx | grep -n -A2 -B2 'disabled\|Button'Repository: hoprnet/node-admin-ui
Length of output: 732
🏁 Script executed:
sed -n '40,150p' src/components/Modal/node/OpenChannelModal.tsxRepository: hoprnet/node-admin-ui
Length of output: 4354
🏁 Script executed:
sed -n '130,160p' src/components/Modal/node/OpenChannelModal.tsxRepository: hoprnet/node-admin-ui
Length of output: 718
🏁 Script executed:
sed -n '140,180p' src/components/Modal/node/OpenMultipleChannelsModal.tsxRepository: hoprnet/node-admin-ui
Length of output: 1306
🏁 Script executed:
sed -n '180,261p' src/components/Modal/node/OpenMultipleChannelsModal.tsxRepository: hoprnet/node-admin-ui
Length of output: 2195
🏁 Script executed:
sed -n '50,130p' src/components/Modal/node/OpenMultipleChannelsModal.tsxRepository: hoprnet/node-admin-ui
Length of output: 2722
Add condition block to prevent duplicate channel open dispatches, especially for OpenMultipleChannelsModal.
The thunk lacks a condition block to prevent duplicate dispatches for the same peer. While OpenChannelModal provides visual feedback by showing a pending state when outgoingOpening[peerAddress] is true, this does not prevent users from re-dispatching (e.g., by clicking the button again or triggering the action via keyboard shortcut).
More critically, OpenMultipleChannelsModal does not use the outgoingOpening state at all. It only disables the button based on canOpen, so users can:
- Click the "Open Channels" button multiple times while pending
- Include the same peer address multiple times in the CSV and trigger concurrent opens for that peer
Recommend adding a condition block to openChannelThunk to guard against opening the same channel when outgoingOpening[destination] is already true, similar to closeChannelThunk.
🤖 Prompt for AI Agents
In `@src/store/slices/node/actionsAsync.ts` around lines 494 - 507, Add a
condition block to openChannelThunk that inspects the current state (RootState)
and cancels duplicate dispatches when a channel to the same peer is already
being opened: in createAsyncThunk('node/openChannel', ...) add a condition
function that reads state.node.outgoingOpening and returns false if
outgoingOpening[payload.destination] (or the appropriate payload.peerAddress
field) is truthy, mirroring the guard used in closeChannelThunk, so repeated
dispatches for the same destination are prevented.
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)
src/future-hopr-lib-components/Navbar/navBar.tsx (1)
170-178: Mobile menu will be empty due to commented-outitemsNavbarprop.The
itemsNavbarprop is commented out on line 172, soNavBarItemswill receive its default empty array. This means the mobile menu will render no navigation items, making it non-functional.If this is intentional for v4, consider removing the mobile menu section entirely. Otherwise, uncomment and restore the prop:
Proposed fix
<div className={`menu mobile ${activaMenu ? ' show-menu' : ''}`}> <NavBarItems - // itemsNavbar={[...itemsNavbarCenter, ...itemsNavbarRight]} + itemsNavbar={[...itemsNavbarCenter, ...itemsNavbarRight]} onButtonClick={() => { setActivaMenu(false); }} mobile /> </div>
🤖 Fix all issues with AI agents
In `@src/future-hopr-lib-components/Brick/index.tsx`:
- Around line 96-102: The Button inside the Brick component renders an anchor
with target="_blank" without rel which is a reverse-tabnabbing risk and
additionally the current conditional renders a link even when buttonHref is
undefined; update the rendering logic in the Brick component so you only render
the anchor Button when both button and buttonHref are present (e.g., condition
on button && buttonHref) and add rel="noopener noreferrer" to the Button anchor;
if button is present but buttonHref is undefined, render a semantic button
element (or the Button component in button mode) instead so both visuals and
semantics remain correct.
In `@src/store/slices/node/actionsAsync.ts`:
- Around line 1080-1095: The reducer assumes
action.payload?.strategy?.strategies is defined and calls .forEach which can
throw; guard it by assigning a safe array first (e.g., const strategies =
action.payload?.strategy?.strategies ?? [] or check Array.isArray(...)) and
iterate over that instead, keeping the existing parsing logic and try/catch,
then set state.configuration.parsedStrategies = parsedStrategies; reference
symbols: action.payload?.strategy?.strategies, parsedStrategies, and
state.configuration.parsedStrategies.
♻️ Duplicate comments (1)
src/store/slices/node/actionsAsync.ts (1)
494-507: Duplicate open-channel guard is still missing.
Same concern as the prior review about preventing repeat dispatches for the same destination.
🧹 Nitpick comments (1)
src/future-hopr-lib-components/Brick/index.tsx (1)
90-94: Addalttext for images (accessibility).Images render without
alt. Consider adding animageAltprop and defaulting totitle(or''for decorative images).♿️ Suggested update
type BrickProps = { title: string; text: React.ReactNode; image: string; + imageAlt?: string; className?: string; reverse?: boolean; centerText?: boolean; noShadow?: boolean; button?: string; buttonHref?: string; }; function Brick({ className = '', reverse = false, centerText, title, image, + imageAlt, noShadow, text, button, buttonHref, }: BrickProps) {<Image className="mobileOnly" src={image} noShadow={noShadow} + alt={imageAlt ?? title} /><Image src={image} noShadow={noShadow} + alt={imageAlt ?? title} />Also applies to: 106-109
| {button && ( | ||
| <Button | ||
| href={props.buttonHref} | ||
| href={buttonHref} | ||
| target="_blank" | ||
| > | ||
| {props.button} | ||
| {button} | ||
| </Button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/future-hopr-lib-components/Brick/index.tsx | head -120Repository: hoprnet/node-admin-ui
Length of output: 186
🏁 Script executed:
# Check if Button component is defined locally or imported
rg "Button" src/future-hopr-lib-components/Brick/index.tsx -B 3 -A 3Repository: hoprnet/node-admin-ui
Length of output: 491
🏁 Script executed:
# Find Button component definition
fd "Button" src/future-hopr-lib-components --type f | head -20Repository: hoprnet/node-admin-ui
Length of output: 229
🏁 Script executed:
# Search for Button component export or definition
rg "export.*Button|function Button|const Button" src/future-hopr-lib-components -A 5Repository: hoprnet/node-admin-ui
Length of output: 4122
🏁 Script executed:
cat -n src/future-hopr-lib-components/Button/index.tsxRepository: hoprnet/node-admin-ui
Length of output: 4018
🏁 Script executed:
cat -n src/future-hopr-lib-components/Brick/index.tsx | head -150Repository: hoprnet/node-admin-ui
Length of output: 3113
Add rel for security with target="_blank" and guard against undefined buttonHref.
Line 99 opens a new tab without rel="noopener noreferrer", creating a reverse-tabnabbing vulnerability. Additionally, button can render even when buttonHref is undefined (since buttonHref is optional in BrickProps), which changes the component from a link to a button element—both should be present for semantic correctness.
🔧 Proposed fix
- {button && (
- <Button
- href={buttonHref}
- target="_blank"
- >
- {button}
- </Button>
- )}
+ {button && buttonHref && (
+ <Button
+ href={buttonHref}
+ target="_blank"
+ rel="noopener noreferrer"
+ >
+ {button}
+ </Button>
+ )}📝 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.
| {button && ( | |
| <Button | |
| href={props.buttonHref} | |
| href={buttonHref} | |
| target="_blank" | |
| > | |
| {props.button} | |
| {button} | |
| </Button> | |
| {button && buttonHref && ( | |
| <Button | |
| href={buttonHref} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| > | |
| {button} | |
| </Button> | |
| )} |
🤖 Prompt for AI Agents
In `@src/future-hopr-lib-components/Brick/index.tsx` around lines 96 - 102, The
Button inside the Brick component renders an anchor with target="_blank" without
rel which is a reverse-tabnabbing risk and additionally the current conditional
renders a link even when buttonHref is undefined; update the rendering logic in
the Brick component so you only render the anchor Button when both button and
buttonHref are present (e.g., condition on button && buttonHref) and add
rel="noopener noreferrer" to the Button anchor; if button is present but
buttonHref is undefined, render a semantic button element (or the Button
component in button mode) instead so both visuals and semantics remain correct.
|
|
||
| let parsedStrategies: ParsedStrategiesType = {}; | ||
|
|
||
| action.payload?.strategy?.strategies.forEach((strategyObj: ParsedStrategiesType) => { | ||
| try { | ||
| const strategyName = Object.keys(strategyObj)[0]; | ||
| if (typeof strategyName !== 'string') return; | ||
| let tmp = strategyObj[strategyName]; | ||
| if (!tmp) return; | ||
| parsedStrategies[strategyName] = tmp; | ||
| } catch (e) { | ||
| console.warn('Error parsing strategy', e); | ||
| } | ||
| }); | ||
|
|
||
| state.configuration.parsedStrategies = parsedStrategies; |
There was a problem hiding this comment.
Guard strategy parsing—.forEach can throw when strategies is undefined.
Optional chaining is missing before the call, so configs without strategies will break this reducer.
🛠️ Proposed fix
- action.payload?.strategy?.strategies.forEach((strategyObj: ParsedStrategiesType) => {
- try {
- const strategyName = Object.keys(strategyObj)[0];
- if (typeof strategyName !== 'string') return;
- let tmp = strategyObj[strategyName];
- if (!tmp) return;
- parsedStrategies[strategyName] = tmp;
- } catch (e) {
- console.warn('Error parsing strategy', e);
- }
- });
+ const strategies = action.payload?.strategy?.strategies;
+ if (Array.isArray(strategies)) {
+ strategies.forEach((strategyObj: ParsedStrategiesType) => {
+ try {
+ const strategyName = Object.keys(strategyObj)[0];
+ if (typeof strategyName !== 'string') return;
+ const tmp = strategyObj[strategyName];
+ if (!tmp) return;
+ parsedStrategies[strategyName] = tmp;
+ } catch (e) {
+ console.warn('Error parsing strategy', e);
+ }
+ });
+ }🤖 Prompt for AI Agents
In `@src/store/slices/node/actionsAsync.ts` around lines 1080 - 1095, The reducer
assumes action.payload?.strategy?.strategies is defined and calls .forEach which
can throw; guard it by assigning a safe array first (e.g., const strategies =
action.payload?.strategy?.strategies ?? [] or check Array.isArray(...)) and
iterate over that instead, keeping the existing parsing logic and try/catch,
then set state.configuration.parsedStrategies = parsedStrategies; reference
symbols: action.payload?.strategy?.strategies, parsedStrategies, and
state.configuration.parsedStrategies.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/store/slices/node/actionsAsync.ts (1)
1279-1311: Fix the constant condition causing the pipeline failure.The
false &&creates an unreachable code block that fails the linter. If the metric parsing should be disabled, either comment out the entire block or use a runtime-configurable flag.🛠️ Suggested fix — comment out the block
// count tickets state.metricsParsed.tickets.incoming = { redeemed: {}, unredeemed: {}, }; - if ( - false && // disable for now as the metric is not available on the test node - jsonMetrics?.hopr_tickets_incoming_statistics?.categories && - jsonMetrics?.hopr_tickets_incoming_statistics?.data - ) { - const categories = jsonMetrics.hopr_tickets_incoming_statistics.categories; - const data = jsonMetrics?.hopr_tickets_incoming_statistics?.data; - for (let i = 0; i < categories.length; i++) { - const channel = categories[i] - .match(/channel="0x[a-f0-9]+"/gi)[0] - .replace(`channel="`, ``) - .replace(`"`, ``); - const statistic = categories[i] - .match(/statistic="[a-z_]+"/g)[0] - .replace(`statistic="`, ``) - .replace(`"`, ``); - const value = data[i]; - - if (value) { - if (statistic === 'unredeemed') { - state.metricsParsed.tickets.incoming.unredeemed[channel] = { - value: `${value}`, - formatted: formatEther(BigInt(`${value}`)), - }; - } else if (statistic === 'redeemed') { - state.metricsParsed.tickets.incoming.redeemed[channel] = { - value: `${value}`, - formatted: formatEther(BigInt(`${value}`)), - }; - } - } - } - } + // TODO: Re-enable when hopr_tickets_incoming_statistics metric is available on the test node + // if ( + // jsonMetrics?.hopr_tickets_incoming_statistics?.categories && + // jsonMetrics?.hopr_tickets_incoming_statistics?.data + // ) { + // const categories = jsonMetrics.hopr_tickets_incoming_statistics.categories; + // const data = jsonMetrics?.hopr_tickets_incoming_statistics?.data; + // for (let i = 0; i < categories.length; i++) { + // const channel = categories[i] + // .match(/channel="0x[a-f0-9]+"/gi)[0] + // .replace(`channel="`, ``) + // .replace(`"`, ``); + // const statistic = categories[i] + // .match(/statistic="[a-z_]+"/g)[0] + // .replace(`statistic="`, ``) + // .replace(`"`, ``); + // const value = data[i]; + // + // if (value) { + // if (statistic === 'unredeemed') { + // state.metricsParsed.tickets.incoming.unredeemed[channel] = { + // value: `${value}`, + // formatted: formatEther(BigInt(`${value}`)), + // }; + // } else if (statistic === 'redeemed') { + // state.metricsParsed.tickets.incoming.redeemed[channel] = { + // value: `${value}`, + // formatted: formatEther(BigInt(`${value}`)), + // }; + // } + // } + // } + // }package.json (1)
19-20: React type definitions must be updated for React 19.The project upgrades to React 19.2.3, but
@types/reactand@types/react-domremain at^18.0.0. React 19 introduces breaking TypeScript changes including:
useRefnow requires an argument- Ref callbacks that return values are now TypeScript errors
ReactElement["props"]defaults tounknowninstead ofany- Global
JSXnamespace removed in favor ofReact.JSXThese type mismatches will cause compilation errors or mask type issues.
🔧 Proposed fix
- "@types/react": "^18.0.0", - "@types/react-dom": "^18.0.0", + "@types/react": "^19.0.0", + "@types/react-dom": "^19.0.0",Additionally, run the React 19 types codemod to address breaking changes:
npx types-react-codemod@latest preset-19 ./srcBased on provided React 19 documentation.
🤖 Fix all issues with AI agents
In `@package.json`:
- Line 15: Update the `@testing-library/react` dependency from "^13.0.0" to
">=16.1.0" in package.json and add `@testing-library/dom` as a dependency (or
devDependency) to satisfy the required peer; ensure you update any lockfile (run
npm install or pnpm install) so the new versions are resolved and adjust any
test imports if needed to accommodate v16 APIs.
♻️ Duplicate comments (2)
src/store/slices/node/actionsAsync.ts (2)
494-507: Add aconditionblock to prevent duplicate channel open dispatches.Unlike
closeChannelThunk, this thunk lacks aconditionguard. Users can trigger concurrent opens for the same peer address by clicking the button multiple times or submitting viaOpenMultipleChannelsModalwith duplicate entries.🛠️ Suggested fix
const openChannelThunk = createAsyncThunk<OpenChannelResponseType, OpenChannelPayloadType, { state: RootState }>( 'node/openChannel', async (payload, { rejectWithValue }) => { try { const res = await openChannel(payload); return res; } catch (e) { if (e instanceof sdkApiError) { return rejectWithValue(e); } return rejectWithValue({ status: JSON.stringify(e) }); } }, + { + condition: (_payload, { getState }) => { + const destination = _payload.destination; + if (!destination) return true; + const isOpening = getState().node.channels.parsed.outgoingOpening[destination]; + if (isOpening) { + return false; + } + }, + }, );
1080-1095: Guard against undefinedstrategiesbefore calling.forEach.Optional chaining on
action.payload?.strategy?doesn't protect againststrategiesbeingundefined. Ifstrategyexists butstrategiesisundefined, calling.forEachwill throw a runtime error.🛠️ Suggested fix
const parsedStrategies: ParsedStrategiesType = {}; - action.payload?.strategy?.strategies.forEach((strategyObj: ParsedStrategiesType) => { - try { - const strategyName = Object.keys(strategyObj)[0]; - if (typeof strategyName !== 'string') return; - const tmp = strategyObj[strategyName]; - if (!tmp) return; - parsedStrategies[strategyName] = tmp; - } catch (e) { - console.warn('Error parsing strategy', e); - } - }); + const strategies = action.payload?.strategy?.strategies; + if (Array.isArray(strategies)) { + strategies.forEach((strategyObj: ParsedStrategiesType) => { + try { + const strategyName = Object.keys(strategyObj)[0]; + if (typeof strategyName !== 'string') return; + const tmp = strategyObj[strategyName]; + if (!tmp) return; + parsedStrategies[strategyName] = tmp; + } catch (e) { + console.warn('Error parsing strategy', e); + } + }); + }
| "@safe-global/protocol-kit": "1.3.0", | ||
| "@safe-global/safe-core-sdk-types": "^2.0.0", | ||
| "@testing-library/jest-dom": "^5.14.1", | ||
| "@testing-library/react": "^13.0.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@testing-library/react React 19 compatibility version requirements
💡 Result:
- React 19 support was added in
@testing-library/reactv16.1.0 (released Dec 5, 2024). The package’s peer dependencies were widened to allowreact/react-dom^18.0.0 || ^19.0.0(and similarly@types/react/@types/react-dom). [1] - If you’re on an older
@testing-library/react(e.g., 13.x–16.0.x), it may require React 18 only (common peer dep error:react@"^18.0.0"). Use@testing-library/react >= 16.1.0when upgrading to React 19. [1] - Extra requirement starting with RTL v16: you must also install
@testing-library/dom(it’s a peer dependency from v16 onward). [2]
Practical minimum for React 19:
react/react-dom:^19.0.0@testing-library/react:>=16.1.0@testing-library/dom: installed alongside (per RTL v16+) [1][2]
Sources
[1] https://github.com/testing-library/react-testing-library/pull/1367/files
[2] https://github.com/testing-library/react-testing-library
Upgrade @testing-library/react to v16.1.0 or higher for React 19 support.
@testing-library/react version ^13.0.0 does not support React 19. React 19 support was added in v16.1.0 (released December 5, 2024). Additionally, starting with v16, @testing-library/dom is a required peer dependency and must also be installed.
🔧 Proposed fix
- "@testing-library/react": "^13.0.0",
+ "@testing-library/react": "^16.1.0",Ensure @testing-library/dom is also added as a dependency alongside this upgrade.
🤖 Prompt for AI Agents
In `@package.json` at line 15, Update the `@testing-library/react` dependency from
"^13.0.0" to ">=16.1.0" in package.json and add `@testing-library/dom` as a
dependency (or devDependency) to satisfy the required peer; ensure you update
any lockfile (run npm install or pnpm install) so the new versions are resolved
and adjust any test imports if needed to accommodate v16 APIs.
1 participant
Summary by CodeRabbit
Chores
Removed
UI / Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.