Conversation
WalkthroughAdds address and chain ID validation utilities and applies them across multiple components to improve vault navigation and parameter handling. Updates InteractionClient with sync tracking and path-parameter resolution. Expands documentation with comprehensive guides for contributing, security testing, and project setup. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/CreateVault/CreateForm.tsx (1)
102-119: Don’t validate withnormalizeAddress()and then submit the raw (possibly invalid) address strings.
Right nowcoincan pass validation (after.trim()), butwriteContract/simulateContractstill receive the untrimmedcoinstring; alsovaultCreatorisn’t validated at all.const validateInputs = () => { const Errors: any = {} if (!coinName) Errors.coinName = 'Name for hodlCoin is required' if (!symbol) Errors.symbol = 'Symbol for hodlCoin is required' - if (!coin || !normalizeAddress(coin)) Errors.coin = 'Valid ERC20 address is required' - if (!vaultCreator) Errors.vaultCreator = 'Vault creator address is required' + const nCoin = normalizeAddress(coin) + if (!nCoin) Errors.coin = 'Valid ERC20 address is required' + const nCreator = normalizeAddress(vaultCreator) + if (!nCreator) Errors.vaultCreator = 'Valid vault creator address is required' if (vaultFee === '') Errors.vaultFee = 'Vault fee is required' if (vaultCreatorFee === '') Errors.vaultCreatorFee = 'Vault creator fee is required' if (Number(vaultFee) < 0) Errors.vaultFee = 'Vault fee cannot be negative' if (Number(vaultCreatorFee) < 0) Errors.vaultCreatorFee = 'Vault creator fee cannot be negative' - if (!isValidChainId(config.state.chainId)) Errors.coin = 'Unsupported chain' + if (!isValidChainId(config.state.chainId)) Errors.chainId = 'Unsupported chain' setErrors(Errors) return Object.keys(Errors).length === 0 }async function createVault() { if (!validateInputs()) { toast({ title: 'Error', description: 'Please fix the errors before submitting.', variant: 'destructive', }); return; } try { setLoadingCreation(true) const chainId = config.state.chainId + const nCoin = normalizeAddress(coin)! + const nCreator = normalizeAddress(vaultCreator)! const tx = await writeContract(config as any, { address: HodlCoinVaultFactories[chainId], abi: HodlCoinFactoryAbi, functionName: 'createHodlCoin', args: [ coinName, symbol, - coin as `0x${string}`, - vaultCreator as `0x${string}`, + nCoin, + nCreator, vaultFeeNumerator, vaultCreatorFeeNumerator, BigInt(stableOrderFee), ], })(Repeat the same
nCoin/nCreatorusage in thesimulateContractcall.)Also applies to: 134-164, 168-181
components/FindVault/FindVaultForm.tsx (1)
67-75: Chain ID UX: resetting to0on invalid/partial input makes the field hard to edit.
Typing an unsupported/partial value immediately collapses to empty (0→''), which can feel “broken”. Consider storingchainIdInput: stringand separately trackingparsedChainId: number | null.Also applies to: 79-79
🧹 Nitpick comments (10)
SECURITY_TEST_REPORT.md (1)
22-44: Userel="noopener noreferrer"(not justnoreferrer) and add test metadata.
Also consider recording the test date, Burp Suite version, and the commit/branch tested so the findings are traceable.- - External Links: Transaction explorer links use `target="_blank"`. Recommend `rel="noreferrer"`. + - External Links: Transaction explorer links use `target="_blank"`. Recommend `rel="noopener noreferrer"`. ... - - Add `rel="noreferrer"` to external links. + - Add `rel="noopener noreferrer"` to external links.CONTRIBUTING.md (1)
41-50: Addnpm run typecheckto the PR checklist (and consider avoiding hardcoded line numbers).## Pull Request Checklist ... - Ensure `npm run lint` passes. +- Ensure `npm run typecheck` passes. - Verify local build if applicable: `npm run build`.components/CreateVault/CreateForm.tsx (1)
440-458: Fortarget="_blank", preferrel="noopener noreferrer".- rel="noreferrer" + rel="noopener noreferrer"README.md (1)
19-33: Docs: vault detail route + scripts list look out of sync with the new routing +typecheckscript.-- Detailed vault view: balances, fees, price, and manual sync (`/v/[vaultId]`) +- Detailed vault view: balances, fees, price, and manual sync (`/[vaultId]?chainId=<id>`) ... ## Scripts ... - `npm run lint`: Run Next.js ESLint checks +- `npm run typecheck`: Run TypeScript type checking (no emit)-``` +```text app/ [vaultId]/ # Vault detail routeAlso applies to: 51-99, 127-133
utils/validation.ts (1)
11-32: Minor robustness/perf: cache supported chain IDs and limitsplitto 2 parts.-import { getSupportedChainIds } from './chains' +import { getSupportedChainIds } from './chains' + +const SUPPORTED_CHAIN_IDS = getSupportedChainIds() export const isValidChainId = (id: number): boolean => { if (!Number.isFinite(id)) return false - return getSupportedChainIds().includes(id) + return SUPPORTED_CHAIN_IDS.includes(id) } ... if (pv.includes(':')) { - const [a, c] = pv.split(':') + const [a, c] = pv.split(':', 2) addr = a cid = Number(c) } else if (pv.includes('@')) { - const [a, c] = pv.split('@') + const [a, c] = pv.split('@', 2) addr = a cid = Number(c) }components/Explorer/CardExplorer.tsx (1)
141-142: Avoidany+ direct style mutation in hover handlers; use proper event typing and CSS/state.
(e: any) => e.currentTarget.style...sidesteps TS and can be brittle across SVG/component changes; preferReact.MouseEvent<SVGSVGElement>(orHTMLElement) and drive hover via className/state.Also applies to: 164-166, 171-172
components/FindVault/FindVaultForm.tsx (2)
31-39: Good validation + routing, butparseChainIdInput(String(chainId))is a smell.
You’re validating a derived string from a numeric state that’s already been coerced; this gets cleaner if you keep the raw input string and parse/validate from that.
88-98: Consider suppressing “Invalid address” while the field is empty.
After a user clears the field, showing an error can be noisy; often better to show errors only when non-empty (or after blur/submit).app/[vaultId]/InteractionClient.tsx (2)
292-298: Prefer an explicitBalancestype overtypeof balancesin state updaters.
UsingsetBalances((prev: typeof balances) => ...)works, but it couples typing to an outer value; a named type/interface is clearer and avoids accidental refactors changing inferred types.Also applies to: 324-328
517-520: Last synced UI: consider including date (not just time) to reduce ambiguity.
If a user leaves the tab open overnight, “Last synced 1:02:03 PM” is unclear without a date.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
CONTRIBUTING.md(1 hunks)README.md(1 hunks)SECURITY_TEST_REPORT.md(1 hunks)app/[vaultId]/InteractionClient.tsx(8 hunks)components/CreateVault/CreateForm.tsx(4 hunks)components/Explorer/CardExplorer.tsx(4 hunks)components/FindVault/FindVaultForm.tsx(4 hunks)package.json(1 hunks)types/shims.d.ts(1 hunks)utils/validation.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
components/Explorer/CardExplorer.tsx (1)
utils/validation.ts (2)
normalizeAddress(5-9)isValidChainId(11-14)
components/FindVault/FindVaultForm.tsx (1)
utils/validation.ts (2)
normalizeAddress(5-9)parseChainIdInput(34-38)
utils/validation.ts (1)
utils/chains.ts (1)
getSupportedChainIds(252-254)
components/CreateVault/CreateForm.tsx (2)
utils/validation.ts (2)
normalizeAddress(5-9)isValidChainId(11-14)utils/config.ts (1)
config(14-27)
app/[vaultId]/InteractionClient.tsx (1)
utils/validation.ts (3)
normalizeAddress(5-9)isValidChainId(11-14)parseVaultPathParam(16-32)
🪛 LanguageTool
CONTRIBUTING.md
[style] ~31-~31: Consider using a different verb for a more formal wording.
Context: ...fig.json:25). - Run npm run lint` and fix issues before committing. - Follow Pret...
(FIX_RESOLVE)
🪛 markdownlint-cli2 (0.18.1)
README.md
52-52: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
package.json (1)
5-11: Good addition—consider wiringnpm run typecheckinto CI / PR checklist.
Having a dedicatedtsc --noEmitstep is useful, but it only helps if it runs consistently (e.g., CI + local checklist).
| useEffect(() => { | ||
| const vault = searchParams.get('vault') | ||
| const chain = searchParams.get('chainId') | ||
|
|
||
| if (vault && chain) { | ||
| setVaultAddress(vault as `0x${string}`) | ||
| setChainId(Number(chain)) | ||
| const naddr = normalizeAddress(vault) | ||
| const cid = Number(chain) | ||
| if (naddr && isValidChainId(cid)) { | ||
| setVaultAddress(naddr) | ||
| setChainId(cid) | ||
| } else { | ||
| setError('Invalid vault address or chain ID') | ||
| } | ||
| return | ||
| } | ||
| const pv = (params as any)?.vaultId | ||
| if (typeof pv === 'string' && pv.length > 0) { | ||
| const { address, chainId: parsedCid } = parseVaultPathParam(pv) | ||
| const cid = parsedCid ?? activeChainId ?? 0 | ||
| if (address && isValidChainId(cid)) { | ||
| setVaultAddress(address) | ||
| setChainId(cid) | ||
| } else { | ||
| setError('Invalid vault address or chain ID') | ||
| } | ||
| } | ||
| }, [searchParams]) | ||
| }, [searchParams, params, activeChainId]) |
There was a problem hiding this comment.
Critical: invalid/missing params can leave the page stuck on the loading screen.
setError(...) in the param-parse effect doesn’t guarantee isLoading becomes false, and the render checks isLoading before error, so users can see an infinite loader instead of the error UI.
useEffect(() => {
const vault = searchParams.get('vault')
const chain = searchParams.get('chainId')
if (vault && chain) {
const naddr = normalizeAddress(vault)
const cid = Number(chain)
if (naddr && isValidChainId(cid)) {
setVaultAddress(naddr)
setChainId(cid)
} else {
setError('Invalid vault address or chain ID')
+ setIsLoading(false)
}
return
}
const pv = (params as any)?.vaultId
if (typeof pv === 'string' && pv.length > 0) {
const { address, chainId: parsedCid } = parseVaultPathParam(pv)
const cid = parsedCid ?? activeChainId ?? 0
if (address && isValidChainId(cid)) {
setVaultAddress(address)
setChainId(cid)
} else {
setError('Invalid vault address or chain ID')
+ setIsLoading(false)
}
+ } else {
+ setError('Missing vault address or chain ID')
+ setIsLoading(false)
}
}, [searchParams, params, activeChainId])- if (isLoading) {
+ if (error) {
+ return (/* existing error UI */)
+ }
+ if (isLoading) {
return (/* existing loading UI */)
}
-
- if (error) {
- return (/* existing error UI */)
- }Also applies to: 401-445
🤖 Prompt for AI Agents
In app/[vaultId]/InteractionClient.tsx around lines 69-95 (and similarly for the
other param parsing block at 401-445), the effect sets setError(...) when params
are invalid but never clears isLoading, so the UI can stay stuck on the loader;
update the effect to call setIsLoading(false) immediately whenever you call
setError (or otherwise ensure isLoading is set to false before returning) so the
error UI can render, and apply the same change to every other code path in these
ranges that sets an error.
| const handleContinue = () => { | ||
| if (vault.vaultAddress) { | ||
| router.push(`/v?chainId=${chainId}&vault=${vault.vaultAddress}`) | ||
| const addr = normalizeAddress(vault.vaultAddress) | ||
| const valid = isValidChainId(chainId) | ||
| if (addr && valid) { | ||
| router.push(`/${addr}?chainId=${chainId}`) | ||
| } | ||
| } |
There was a problem hiding this comment.
Add user-visible feedback when navigation is blocked by validation (currently no-op).
Right now, invalid vault.vaultAddress / chainId just does nothing; consider a toast so users aren’t stuck clicking “Enter Vault” with no result.
🤖 Prompt for AI Agents
In components/Explorer/CardExplorer.tsx around lines 49-55, the handleContinue
function silently returns when normalizeAddress or isValidChainId fails; change
it to show user-visible feedback (e.g., a toast or inline error) for each
validation failure before returning. Specifically, import/use your app’s toast
utility (or set an error state) and call toast.error("Invalid vault address")
when addr is falsy and toast.error("Unsupported chain") when valid is false;
keep the existing router.push only when both validations pass.
| declare module 'lucide-react'; | ||
| declare module 'wagmi'; | ||
| declare module '@wagmi/core'; | ||
| declare module 'next/navigation'; | ||
| declare module 'next/link'; | ||
| declare module 'react' { | ||
| export function useState<T>(initialState: T | (() => T)): [T, (value: T | ((prev: T) => T)) => void]; | ||
| export function useEffect(effect: (...args: any[]) => any, deps?: any[]): void; | ||
| export function useCallback<T extends (...args: any[]) => any>(callback: T, deps: any[]): T; | ||
| } | ||
|
|
||
| declare namespace JSX { | ||
| interface IntrinsicElements { | ||
| [elemName: string]: any; | ||
| } | ||
| } | ||
|
|
||
| declare namespace React { | ||
| type MouseEvent = any; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if package.json exists and what dependencies are listed
find . -name "package.json" -type f | head -5Repository: StabilityNexus/hodlCoin-Solidity-WebUI
Length of output: 96
🏁 Script executed:
cat ./package.json | head -80Repository: StabilityNexus/hodlCoin-Solidity-WebUI
Length of output: 1750
🏁 Script executed:
cat ./tsconfig.jsonRepository: StabilityNexus/hodlCoin-Solidity-WebUI
Length of output: 768
🏁 Script executed:
# Check if these packages actually have type definitions
# Let's look for .d.ts files or types field in package.json for key packages
cd node_modules
for pkg in lucide-react wagmi @wagmi/core; do
if [ -f "$pkg/package.json" ]; then
echo "=== $pkg ==="
cat "$pkg/package.json" | grep -A 2 '"types"\|"typings"' | head -5
fi
doneRepository: StabilityNexus/hodlCoin-Solidity-WebUI
Length of output: 144
🏁 Script executed:
# Check the actual shims file content more carefully
cat -n ./types/shims.d.tsRepository: StabilityNexus/hodlCoin-Solidity-WebUI
Length of output: 822
🏁 Script executed:
# Search for any comments or issues in the codebase explaining why these shims were needed
rg -i "shim|type.*issue|@wagmi|lucide" --type ts --type tsx -B 2 -A 2 | head -100Repository: StabilityNexus/hodlCoin-Solidity-WebUI
Length of output: 109
🏁 Script executed:
# Search for any references to shims or type issues
rg -i "shim|types/shims" --type ts | head -20Repository: StabilityNexus/hodlCoin-Solidity-WebUI
Length of output: 64
🌐 Web query:
lucide-react TypeScript definitions type support
💡 Result:
Short answer
- lucide-react ships bundled TypeScript definitions: each icon is a typed React component and the package exports types like LucideProps and LucideIcon. [1][2]
What that means
- Icons are typed as React forward-ref components: ForwardRefExoticComponent<Omit<LucideProps, "ref"> & RefAttributes>. LucideProps includes common SVG props plus size and absoluteStrokeWidth. [2]
- Docs explicitly state “full TypeScript support” for lucide-react. [1]
Common problems & fixes
- If you get JSX/element-type errors (e.g. “X cannot be used as a JSX component”), it’s usually a TypeScript / @types/react mismatch (React 18+ vs TS / JSX settings) — resolve by aligning TS, @types/react/@types/react-dom (or using the built-in react types with TS 4.5+/React 18), or relax type checking (e.g. skipLibCheck) as a temporary workaround. See community reports and discussion. [3]
If you want, tell me:
- your TS version, @types/react (or react) version, and tsconfig.jsx setting and I’ll give precise fixes and config snippets.
Sources
- lucide.dev — Lucide React package docs. [1]
- Bundled declaration file (lucide-react.d.ts). [2]
- GitHub issue showing typical JSX/TS errors and discussion. [3]
🌐 Web query:
wagmi @wagmi/core TypeScript definitions type support
💡 Result:
Short answer — @wagmi/core has first-class TypeScript support. Key points:
- Requirements: TypeScript (docs state >= 5.7.3) and strict mode enabled in tsconfig. [1]
- Type inference: wagmi infers types from ABIs and EIP‑712 Typed Data (powered by Viem / ABIType). To get inference you must define ABIs/typed data inline or use const assertions (e.g.,
const abi = [...] as const). [1][2] - Package dependency: install/use [email protected] alongside @wagmi/core for the TypeScript integration. [3]
- Stability notes: type changes may be released as patch updates; lock @wagmi/core and typescript to specific patch versions for reproducible behavior. [1]
If you want, I can show a minimal TypeScript example (readContract / signTypedData) demonstrating const assertions and correct tsconfig.
🌐 Web query:
Next.js next/navigation next/link TypeScript types built-in
💡 Result:
Short answer: Yes — Next.js ships built-in TypeScript types for next/link and next/navigation, and recent Next.js versions provide optional statically-typed routes so Link.href and the App Router navigation methods (push/replace/prefetch) are type-checked.
What to know and how to enable it
- Next.js includes TypeScript support and its type definitions by default. [1]
- To get statically-typed links you opt into the typed-routes feature (named experimental.typedRoutes or typedRoutes depending on Next.js version) in next.config and use TypeScript; Next.js then generates route types under .next/types which you should include in tsconfig.json. This types Link.href and App Router navigation (useRouter/useNavigation) for literal routes; non-literal hrefs may need a cast (as Route). [2][3]
Example (next.config):
- next.config.ts / next.config.js: enable typedRoutes/experimental.typedRoutes, then run next dev/build so .next/types is generated and picked up by TypeScript. Include ".next/types/**/*.ts" in tsconfig.json include if needed. [2][3]
References
- Next.js TypeScript docs (statically typed links, generated .next/types). [2]
- Next.js TypeScript config / general TypeScript support. [1][3]
🌐 Web query:
React 18 useState useEffect useCallback TypeScript @types/react correct types
💡 Result:
Short summary and correct typings (React 18 + TypeScript / @types/react)
-
useState
- Signature: function useState
(initialState: S | (() => S)): [S, Dispatch<SetStateAction>] - Usage: prefer explicit generic when initial value is null/unknown: const [user, setUser] = useState<User | null>(null). [1][2]
- Signature: function useState
-
useEffect / useLayoutEffect
- Signature: function useEffect(effect: () => (void | (() => void | undefined)), deps?: DependencyList)
- Notes: effect may return a cleanup function or void; deps is DependencyList (readonly any[]). [1][2]
-
useCallback
- React 18 signature: function useCallback(callback: T, deps: DependencyList): T
- Implication: callback parameter types are not forced by useCallback’s type; you must type the callback parameters explicitly (or provide a generic) to avoid implicit any. Example:
- const handle = useCallback((e: React.MouseEvent) => { ... }, []);
- or const handle = useCallback<() => void>(() => { ... }, []);
- Older React @types used T extends (...args: any[]) => any; React 18 widened to T extends Function which can surface implicit-any on params. [1][2][3]
Recommendations
- Install matching @types/react for your React version.
- Explicitly annotate state generics when initial value is null/unknown.
- Always type callback parameters for useCallback to satisfy strict TS settings.
- Hover/signature in VS Code or inspect @types/react in node_modules for exact local types.
Sources: React docs (react.dev) and React + TypeScript cheatsheet / @types/react notes. [1][2][3]
References
[1] React docs — Using TypeScript (react.dev).
[2] React + TypeScript Cheatsheet (react-typescript-cheatsheet).
[3] typescript-cheatsheets / react GitHub notes on useCallback and hook signatures.
🏁 Script executed:
# Let's check if there are any files importing from 'react' that might rely on these hook signatures
rg "useState|useEffect|useCallback" --type ts -l | head -10Repository: StabilityNexus/hodlCoin-Solidity-WebUI
Length of output: 380
🏁 Script executed:
# Check one of the files that uses hooks to see the actual usage pattern
head -40 ./components/hooks/useTokenSearch.tsRepository: StabilityNexus/hodlCoin-Solidity-WebUI
Length of output: 1352
🏁 Script executed:
# Check the rest of the file to see actual useState/useEffect usage
tail -50 ./components/hooks/useTokenSearch.tsRepository: StabilityNexus/hodlCoin-Solidity-WebUI
Length of output: 1451
🏁 Script executed:
# Get full file to see useState usage
wc -l ./components/hooks/useTokenSearch.ts
cat ./components/hooks/useTokenSearch.ts | grep -A 3 "useState"Repository: StabilityNexus/hodlCoin-Solidity-WebUI
Length of output: 538
Remove these type shims—they mask real types and some signatures are incorrect.
The packages declared in lines 1–5 (lucide-react, wagmi, @wagmi/core, next/navigation, next/link) all ship with first-class TypeScript definitions, making those ambient module declarations unnecessary and harmful. Declaring them as untyped silences real type errors.
Additionally:
- Line 8 (useEffect signature): The
(...args: any[]) => anyis wrong; React 18's useEffect expects() => void | (() => void). This incorrect override could mask legitimate type issues in effect handlers. - Lines 12–15 (JSX.IntrinsicElements): Widening to
[elemName: string]: anydefeats JSX prop validation. - Line 19 (React.MouseEvent): Setting to
anyremoves event type safety.
These shims undermine the value of npm run typecheck by hiding real issues.
Recommended action: Remove types/shims.d.ts entirely and verify npm run typecheck passes. If it does, the shims were not needed. If it fails, fix the underlying issues rather than adding blanket any declarations.
🤖 Prompt for AI Agents
types/shims.d.ts lines 1–20: delete this shim file entirely (it masks real type
definitions for lucide-react, wagmi, @wagmi/core, next/navigation, next/link and
overrides React types incorrectly); after removal run npm run typecheck and fix
any resulting type errors in code (prefer correcting code or adding
narrowly-scoped declaration files with accurate types rather than reintroducing
blanket any shims), ensure React hooks and event types use their proper
signatures (useEffect returns void | (() => void), restore proper
JSX.IntrinsicElements typings and remove the React.MouseEvent:any override).
3 participants
Fix Routing & Strengthen Input Validation Across Vault Workflows
#81
Summary
This PR standardizes vault routing and introduces robust, reusable input validation to improve reliability, security, and UX across all vault-related flows.
Key Changes
Routing Standardization
app/[vaultId]/${address}?chainId=<id>Centralized Validation
utils/validation.tswith helpers for:Validation Integration
UI Improvements
Security
SECURITY_TEST_REPORT.mddocumenting Burp Suite testing methodology, findings, and recommendationsCode References
app/[vaultId]/InteractionClient.tsx:66components/FindVault/FindVaultForm.tsx:60components/Explorer/CardExplorer.tsx:49components/CreateVault/CreateForm.tsx:101