Refactor Injective integration: Centralize constants and improve erro… #1
Conversation
WalkthroughThis pull request integrates Injective blockchain wallet support into the application. It adds SDK dependencies, introduces a wallet provider context for managing Injective wallet connections, adds a "Keplr (Injective)" login option, creates a new portfolio display component for Injective assets, and extends the profile feed to show Injective-specific data. Changes
Sequence DiagramsequenceDiagram
actor User
participant WalletSelector as WalletSelector Component
participant Provider as InjectiveWalletProvider
participant SDK as Injective SDK/Chain
participant Portfolio as InjectivePortfolio
User->>WalletSelector: Click "Keplr (Injective)" button
WalletSelector->>Provider: Call connectInjective(Wallet.Keplr)
Provider->>SDK: Initialize wallet strategy & connect
SDK-->>Provider: Return wallet address
Provider->>Provider: Store address in state
Provider-->>WalletSelector: Connection complete
Note over User,Portfolio: User navigates to Profile with Injective feedType
Portfolio->>Provider: Read address from context
Portfolio->>SDK: Fetch portfolio via IndexerGrpcAccountApi
SDK-->>Portfolio: Return bankBalances & subaccounts
Portfolio->>Portfolio: Render assets list
Portfolio-->>User: Display "Injective Assets"
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/Profile/index.tsx (1)
41-53: Critical: Injective feed type missing from URL routing logic.The
lowerCaseProfileFeedTypearray (lines 41-48) does not includeProfileFeedType.Injective.toLowerCase(), which means:
- Direct URL navigation with
?type=injectivewill not be recognized- The feed type will fallback to
ProfileFeedType.Feedinstead of honoring the Injective tab- Users cannot share or bookmark direct links to the Injective portfolio view
This breaks expected URL-based navigation for the new Injective tab.
Apply this diff:
const lowerCaseProfileFeedType = [ ProfileFeedType.Feed.toLowerCase(), ProfileFeedType.Replies.toLowerCase(), ProfileFeedType.Media.toLowerCase(), ProfileFeedType.Collects.toLowerCase(), ProfileFeedType.Nft.toLowerCase(), - ProfileFeedType.Stats.toLowerCase() + ProfileFeedType.Stats.toLowerCase(), + ProfileFeedType.Injective.toLowerCase() ];
🧹 Nitpick comments (11)
apps/web/src/components/Profile/FeedType.tsx (1)
85-85: Consider using a unique icon for the Injective tab.The
CollectionIconis already used by the "Collected" tab (line 62), which may cause visual confusion for users navigating between tabs.apps/web/src/components/Common/Providers/index.tsx (1)
70-72: Provider integration looks good.The
InjectiveWalletProvideris correctly positioned in the provider hierarchy. Note that the provider implementation (InjectiveWalletProvider.tsx, line 29) contains a TODO about makingchainIdconfigurable via environment variables, which should be addressed for production flexibility.Based on the TODO in InjectiveWalletProvider.tsx, do you want me to help implement environment-based chain ID configuration?
packages/data/constants.ts (1)
17-21: Consider environment-aware endpoint configuration.The
INJECTIVE_ENDPOINTSare hardcoded to mainnet URLs, unlike other endpoints in this file which useIS_MAINNETchecks (e.g.,POLYGONSCAN_URLat lines 69-71,SNAPSHOT_HUB_URLat lines 31-33). This limits flexibility for testing and development environments.Consider making these configurable:
export const INJECTIVE_ENDPOINTS = { - indexerApi: 'https://sentry.exchange.grpc-web.injective.network', - grpc: 'https://sentry.chain.grpc-web.injective.network', - rest: 'https://sentry.lcd.injective.network' + indexerApi: IS_MAINNET + ? 'https://sentry.exchange.grpc-web.injective.network' + : 'https://testnet.sentry.exchange.grpc-web.injective.network', + grpc: IS_MAINNET + ? 'https://sentry.chain.grpc-web.injective.network' + : 'https://testnet.sentry.chain.grpc-web.injective.network', + rest: IS_MAINNET + ? 'https://sentry.lcd.injective.network' + : 'https://testnet.lcd.injective.network' };apps/web/src/components/Profile/InjectivePortfolio.tsx (3)
9-14: Optimize API client instantiation.The
IndexerGrpcAccountApiis instantiated on every call tofetchBalances. Consider moving it outside the function or memoizing it to avoid unnecessary object creation.+const indexerAccountApi = new IndexerGrpcAccountApi(INJECTIVE_ENDPOINTS.indexerApi); + const fetchBalances = async (address: string) => { - const indexerAccountApi = new IndexerGrpcAccountApi(INJECTIVE_ENDPOINTS.indexerApi); const accountPortfolio = await indexerAccountApi.fetchAccountPortfolio(address); return accountPortfolio; };
82-82: Unused data: subaccountsList is fetched but never displayed.Line 82 extracts
subaccountBalancesfrom the portfolio, but this data is never rendered in the UI (lines 84-106 only display bank balances). Either display subaccount balances or remove the unused extraction.
96-101: Consider formatting raw denomination values.The
balance.denomvalues are displayed as raw strings (e.g., "uinj", "inj"), which may not be user-friendly. Consider formatting or mapping these to human-readable asset names and applying proper decimal formatting to the amounts based on token decimals.apps/web/src/lib/InjectiveWalletProvider.tsx (5)
1-12: Alias InjectiveWalletto avoid confusion with GraphQLWallettype and model disconnected state explicitlyGiven the existing
WalletGraphQL type inpackages/lens/generated.ts, importingWalletfrom@injective-labs/wallet-strategyunder the same name can be confusing when both need to be used in the same module. Consider aliasing it for clarity and to avoid future import collisions, and make the disconnected state explicit in the type instead of using an empty string sentinel.-import { Wallet, WalletStrategy } from '@injective-labs/wallet-strategy'; +import { Wallet as InjectiveWallet, WalletStrategy } from '@injective-labs/wallet-strategy'; -interface InjectiveWalletContextType { - walletStrategy: WalletStrategy; - address: string; - chainId: ChainId; - connect: (wallet: Wallet) => Promise<void>; +interface InjectiveWalletContextType { + walletStrategy: WalletStrategy; + address: string | null; + chainId: ChainId; + connect: (wallet: InjectiveWallet) => Promise<void>; disconnect: () => void; isLoading: boolean; }You’d then update the rest of this file and consumers to use
InjectiveWalletand treatnullas “not connected”.
24-37: HardenchainIdenv handling and align the TODO with current behaviorRight now
chainIdis cast fromprocess.env.NEXT_PUBLIC_INJECTIVE_CHAIN_IDand falls back toChainId.Mainnet, but invalid values flow through silently. Consider validating the env value against knownChainIdmembers and logging/warning when it’s not recognized, so misconfiguration doesn’t quietly point to the wrong network. Also, the TODO about makingchainIdconfigurable via env is effectively already implemented; you may want to rephrase it toward runtime network switching or remove it to avoid confusion.- // Initialize Wallet Strategy - // TODO: Make chainId configurable via env - const chainId = (process.env.NEXT_PUBLIC_INJECTIVE_CHAIN_ID as unknown as ChainId) || ChainId.Mainnet; + // Initialize Wallet Strategy + // TODO: Support runtime network switching beyond a single env-configured chain + const envChainId = process.env.NEXT_PUBLIC_INJECTIVE_CHAIN_ID; + const chainId: ChainId = + (envChainId && (ChainId as unknown as Record<string, ChainId>)[envChainId]) ?? ChainId.Mainnet;(Exact mapping may need to be adjusted based on how
ChainIdis defined in@injective-labs/ts-types.)
38-51: Handle empty address arrays and expose error/connection result to callersIf
walletStrategy.getAddresses()ever returns an empty array, the previousaddressvalue is left intact, which can result in stale state. Also, callers have no way to distinguish between a successful connection and a silent failure. Consider resettingaddressoptimistically and returning a boolean or throwing to let the UI react.- const connect = async (wallet: Wallet) => { + const connect = async (wallet: InjectiveWallet): Promise<boolean> => { setIsLoading(true); try { - walletStrategy.setWallet(wallet); - const addresses = await walletStrategy.getAddresses(); - if (addresses.length > 0) { - setAddress(addresses[0]); - } + walletStrategy.setWallet(wallet); + setAddress(null); + + const addresses = await walletStrategy.getAddresses(); + if (addresses.length > 0) { + setAddress(addresses[0]); + return true; + } + return false; } catch (error) { console.error('Failed to connect Injective wallet:', error); + setAddress(null); + return false; } finally { setIsLoading(false); } };This keeps state consistent and lets consumers decide how to handle failed or empty connections.
53-57: Consider delegating disconnect/cleanup towalletStrategyif supportedRight now
disconnectonly clears local state; ifWalletStrategymaintains subscriptions, event listeners, or internal wallet references, they may never be cleaned up. If the library exposes adisconnect/reset/clearWallet-style method, it would be safer to call that here as well.- const disconnect = () => { - setAddress(''); - // WalletStrategy doesn't strictly "disconnect" in the same way, - // but we clear local state. - }; + const disconnect = () => { + // If WalletStrategy exposes a reset/disconnect API, call it here as well. + try { + // Example: walletStrategy.disconnect?.(); + } catch (error) { + console.error('Failed to disconnect Injective wallet:', error); + } finally { + setAddress(null); + } + };Adjust the internal call once you confirm the appropriate cleanup API in
@injective-labs/wallet-strategy.
59-70: Optional: Memoize the context value to avoid unnecessary re-rendersThe provider currently creates a new context
valueobject on every render, which will re-render all consumers even when fields they care about haven’t changed. This is totally fine for now, but if you see many consumers or noticeable re-render costs, wrapping the value inuseMemoand the callbacks inuseCallbackwould help.- return ( - <InjectiveWalletContext.Provider value={{ - walletStrategy, - address, - chainId, - connect, - disconnect, - isLoading - }}> + const contextValue = useMemo( + () => ({ + walletStrategy, + address, + chainId, + connect, + disconnect, + isLoading, + }), + [walletStrategy, address, chainId, connect, disconnect, isLoading], + ); + + return ( + <InjectiveWalletContext.Provider value={contextValue}> {children} </InjectiveWalletContext.Provider> );This is a micro-optimization and can be deferred unless performance becomes a concern.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/web/package.json(1 hunks)apps/web/src/components/Common/Providers/index.tsx(2 hunks)apps/web/src/components/Profile/FeedType.tsx(1 hunks)apps/web/src/components/Profile/InjectivePortfolio.tsx(1 hunks)apps/web/src/components/Profile/index.tsx(2 hunks)apps/web/src/components/Shared/Login/WalletSelector.tsx(3 hunks)apps/web/src/enums.ts(1 hunks)apps/web/src/lib/InjectiveWalletProvider.tsx(1 hunks)packages/data/constants.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/src/components/Common/Providers/index.tsx (1)
apps/web/src/lib/InjectiveWalletProvider.tsx (1)
InjectiveWalletProvider(24-71)
apps/web/src/lib/InjectiveWalletProvider.tsx (1)
packages/lens/generated.ts (1)
Wallet(4681-4686)
🔇 Additional comments (5)
apps/web/src/components/Profile/index.tsx (1)
161-161: LGTM: Injective portfolio rendering.The conditional rendering logic for
InjectivePortfoliois correct and consistent with other feed type renderings.apps/web/src/enums.ts (1)
20-21: LGTM: Enum addition.The new
Injectiveenum member is correctly added toProfileFeedType.packages/data/constants.ts (1)
17-21: Injective endpoint URLs are correct and active.All three endpoints match the official Injective mainnet public endpoints documented in the Injective Docs:
indexerApiuses the correct Indexer gRPC-Web endpointgrpcuses the correct Chain gRPC-Web endpointrestuses the correct REST/LCD endpointThe naming convention aligns with TS SDK specifications. No changes needed.
apps/web/src/components/Profile/InjectivePortfolio.tsx (1)
16-24: Add explicit return type tofetchBalancesfunction.The
fetchBalancesfunction returnsaccountPortfoliowithout an explicit return type annotation. Consider importing and using the SDK's proper type definition (likelyAccountPortfoliofrom@injective-labs/sdk-ts) instead of the localPortfoliointerface to ensure type safety and consistency with the underlying SDK.apps/web/src/lib/InjectiveWalletProvider.tsx (1)
14-22: Hook pattern and safety check look goodThe
useInjectiveWallethook with a null-guard and explicit error when used outside the provider is a solid, safe pattern and should make misconfigurations fail fast during development.
| "@injective-labs/sdk-ts": "^1.10.45", | ||
| "@injective-labs/ts-types": "^1.10.45", | ||
| "@injective-labs/wallet-strategy": "^1.10.45", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@injective-labs/sdk-ts latest version npm
💡 Result:
Latest npm version: @injectivelabs/sdk-ts v1.16.10 (as of Dec 9, 2025). [1][2]
Install: npm install @injectivelabs/sdk-ts@latest or npm i @injectivelabs/sdk-ts. [1]
Sources:
- npm package page. [1]
- InjectiveLabs GitHub / docs (package repo). [2]
🌐 Web query:
@injective-labs/ts-types latest version npm
💡 Result:
The latest published npm version of @injective-labs/ts-types is 1.16.37. [1][2]
Sources:
- jsDelivr/npm package listing showing version 1.16.37. [1]
- Socket.dev package page (npm metadata) listing recent versions. [2]
🌐 Web query:
@injective-labs/wallet-strategy latest version npm
💡 Result:
Latest npm version of @injective-labs/wallet-strategy is 1.16.7 (published 2 days ago). [1][2]
Sources:
- npm package page. [1]
- Injective docs (wallet strategy). [2]
🌐 Web query:
@injective-labs packages security vulnerabilities CVE 2025
💡 Result:
I searched NVD, GitHub security/advisories, and npm for 2025 CVEs affecting @injective-labs packages and found no CVE entries or vendor advisories tied to Injective Labs packages for 2025.
Sources checked:
- NVD (National Vulnerability Database) search (no CVE for Injective Labs). [1]
- InjectiveLabs GitHub repos (injective-ts, sdk-go, others) — no published security advisories/CVEs listed. [2][3]
- npm package listing for @InjectiveLabs (example: @injectivelabs/networks) — no malware/security advisory on the package page. [4]
If you want, I can:
- perform a deeper scan (package-by-package) across the InjectiveLabs npm scope and their GitHub repos and return any historical advisories or dependency CVEs; or
- set up alerts/notify if any new CVE appears for these packages.
References:
[1] NVD — National Vulnerability Database.
[2] GitHub — InjectiveLabs/injective-ts.
[3] GitHub — InjectiveLabs/sdk-go.
[4] npm — @injectivelabs/networks.
Update Injective packages to current versions.
The pinned versions are significantly outdated:
@injective-labs/sdk-ts: ^1.10.45 → latest v1.16.10@injective-labs/ts-types: ^1.10.45 → latest v1.16.37@injective-labs/wallet-strategy: ^1.10.45 → latest v1.16.7
While no security vulnerabilities were found for these versions, updating to the latest releases will ensure access to bug fixes, performance improvements, and compatibility enhancements.
| /> | ||
| )} | ||
| <TabButton | ||
| name="Injective" |
There was a problem hiding this comment.
Missing localization for Injective tab name.
The tab name is hardcoded as "Injective" while all other tabs use the t macro for internationalization. This creates inconsistency with the established localization pattern.
Apply this diff:
- name="Injective"
+ name={t`Injective`}📝 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.
| name="Injective" | |
| name={t`Injective`} |
🤖 Prompt for AI Agents
In apps/web/src/components/Profile/FeedType.tsx around line 84, the tab label is
hardcoded as "Injective" while other tabs use the t(...) localization macro;
replace the hardcoded string with the same t call pattern used elsewhere (e.g.,
t('profile.tabs.injective') or the existing key convention), and if that
translation key does not exist add it to the appropriate i18n translation files
for all supported locales.
| useEffect(() => { | ||
| if (!address) return; | ||
|
|
||
| const loadBalances = async () => { | ||
| setLoading(true); | ||
| setError(null); | ||
| try { | ||
| const data = await fetchBalances(address); | ||
| setPortfolio(data); | ||
| } catch (err: any) { | ||
| setError(new Error(err?.message || 'Failed to fetch balances')); | ||
| console.error('Failed to fetch Injective balances', err); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| loadBalances(); | ||
| }, [address]); |
There was a problem hiding this comment.
Memory leak risk: missing cleanup in async useEffect.
The useEffect performs an async operation but doesn't handle component unmount. If the component unmounts while loadBalances is in progress, the state updates on lines 40 and 45 will execute on an unmounted component, potentially causing memory leaks and React warnings.
Apply this diff to add cleanup:
useEffect(() => {
if (!address) return;
+
+ let isMounted = true;
const loadBalances = async () => {
setLoading(true);
setError(null);
try {
const data = await fetchBalances(address);
- setPortfolio(data);
+ if (isMounted) {
+ setPortfolio(data);
+ }
} catch (err: any) {
- setError(new Error(err?.message || 'Failed to fetch balances'));
- console.error('Failed to fetch Injective balances', err);
+ if (isMounted) {
+ setError(new Error(err?.message || 'Failed to fetch balances'));
+ console.error('Failed to fetch Injective balances', err);
+ }
} finally {
- setLoading(false);
+ if (isMounted) {
+ setLoading(false);
+ }
}
};
loadBalances();
+
+ return () => {
+ isMounted = false;
+ };
}, [address]);📝 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.
| useEffect(() => { | |
| if (!address) return; | |
| const loadBalances = async () => { | |
| setLoading(true); | |
| setError(null); | |
| try { | |
| const data = await fetchBalances(address); | |
| setPortfolio(data); | |
| } catch (err: any) { | |
| setError(new Error(err?.message || 'Failed to fetch balances')); | |
| console.error('Failed to fetch Injective balances', err); | |
| } finally { | |
| setLoading(false); | |
| } | |
| }; | |
| loadBalances(); | |
| }, [address]); | |
| useEffect(() => { | |
| if (!address) return; | |
| let isMounted = true; | |
| const loadBalances = async () => { | |
| setLoading(true); | |
| setError(null); | |
| try { | |
| const data = await fetchBalances(address); | |
| if (isMounted) { | |
| setPortfolio(data); | |
| } | |
| } catch (err: any) { | |
| if (isMounted) { | |
| setError(new Error(err?.message || 'Failed to fetch balances')); | |
| console.error('Failed to fetch Injective balances', err); | |
| } | |
| } finally { | |
| if (isMounted) { | |
| setLoading(false); | |
| } | |
| } | |
| }; | |
| loadBalances(); | |
| return () => { | |
| isMounted = false; | |
| }; | |
| }, [address]); |
🤖 Prompt for AI Agents
In apps/web/src/components/Profile/InjectivePortfolio.tsx around lines 32 to 50,
the useEffect starts an async loadBalances that updates state but doesn't guard
against component unmount, risking state updates on an unmounted component; fix
by adding a mounted flag (let isMounted = true) or an AbortController if
fetchBalances supports a signal, pass the signal to fetchBalances (or check
isMounted) and before every setLoading, setError, and setPortfolio ensure
isMounted is true (or abort early on signal), and return a cleanup function that
sets isMounted = false (or calls controller.abort()) to prevent state updates
after unmount.
| <img | ||
| src="https://keplr.app/img/keplr.png" | ||
| draggable={false} | ||
| className="h-6 w-6" | ||
| height={24} | ||
| width={24} | ||
| alt="Keplr" | ||
| /> |
There was a problem hiding this comment.
Reliability concern: external image URL and missing error handling.
Two issues:
- The Keplr logo uses an external URL (
https://keplr.app/img/keplr.png) which creates a single point of failure. If the external domain is unavailable or the image path changes, the UI will break. - The
connectInjectivecall lacks error handling. Unlike the standard wallet connectors which have error handling via theuseConnecthook (line 61), failures in Injective wallet connection won't be caught or displayed to users.
Recommended fixes:
- Host the Keplr logo locally in your static assets directory
- Wrap the
onClickhandler with try-catch to handle connection errors:
- onClick={() => connectInjective(Wallet.Keplr)}
+ onClick={async () => {
+ try {
+ await connectInjective(Wallet.Keplr);
+ } catch (err: any) {
+ errorToast(err);
+ }
+ }}Committable suggestion skipped: line range outside the PR's diff.
1 participant
Refactored Injective Portfolio Integration: Improving Maintainability and UX
This PR completely refactors the Injective portfolio integration component to significantly enhance code quality, maintainability, and the overall user experience (UX).
Key Improvements
Feature | Details | Benefit -- | -- | -- Centralized Configuration | Moved hardcoded Injective API endpoints (Indexer, gRPC, REST) from InjectivePortfolio.tsx into a reusable INJECTIVE_ENDPOINTS constant in packages/data/constants.ts. | Simplifies maintenance and promotes configuration consistency. Enhanced Error Handling | Implemented robust error handling logic. Failures now display a user-friendly ErrorMessage component. | Eliminates silent failures and significantly improves the user experience during network issues. Localization Ready | All static text strings were wrapped using the t macro and components. | Prepares the component for immediate internationalization (i18n) support. Type Safety | Cleaned up and refined the TypeScript interfaces for Portfolio and Coin data structures. | Ensures stronger type guarantees and improves code readability.Verification
The following tests confirm there are no regressions:
Injective assets load correctly when a wallet is connected.
The "Connect Wallet" empty state displays correctly.
Network errors successfully trigger the new, enhanced error UI state.
The build process has been confirmed to be regression-free.
Ready for review! Please provide any feedback on the structural and functional changes.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.