[Refactor] unify create pool validation with zod#90
Open
kushbosamiya wants to merge 6 commits intoStabilityNexus:mainfrom
Open
[Refactor] unify create pool validation with zod#90kushbosamiya wants to merge 6 commits intoStabilityNexus:mainfrom
kushbosamiya wants to merge 6 commits intoStabilityNexus:mainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughIntroduce Zod-based per-step form schemas and centralized validation messages; remove duplicate validators; adjust linting and deps; switch Google fonts out; replace native img with Next.js Image; tweak wagmi fallback/config; add Next image remote patterns; coerce chart tooltip values to numbers. Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User (browser)"
participant Form as "CreateFatePool component"
participant Schema as "Zod Step Schema"
participant Messages as "validationMessages"
participant UI as "Form UI (errors)"
User->>Form: submit current step
Form->>Schema: schema.safeParse(formData)
Schema-->>Form: success / error issues
alt success
Form->>UI: advance step / clear errors
else error
Form->>Messages: map issues -> localized keys
Messages-->>Form: message strings
Form->>UI: display field errors
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Forms/Steps/PoolConfigurationStep.tsx (1)
43-63:⚠️ Potential issue | 🟡 MinorUnnecessary token reload on
baseTokenAddresschange.Adding
formData.baseTokenAddressto this dependency array causesloadTokensForChain(chainId)to re-run whenever the base token address changes, even though the token list depends only onchainId. The seconduseEffect(lines 66-73) already handles updatingselectedTokenwhenbaseTokenAddresschanges.This creates redundant async calls and could cause unnecessary re-renders. Consider removing
formData.baseTokenAddressfrom this dependency array.🐛 Proposed fix
loadTokens(); - }, [chainId, formData.baseTokenAddress]); + }, [chainId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Forms/Steps/PoolConfigurationStep.tsx` around lines 43 - 63, The useEffect that defines loadTokens currently lists formData.baseTokenAddress in its dependency array causing unnecessary reloads; remove formData.baseTokenAddress from that dependency array so the effect only depends on chainId, since loadTokensForChain(chainId) and setAvailableTokens only need chainId, and the separate effect that watches formData.baseTokenAddress (which calls setSelectedToken) will handle updating the selection; update the dependency array on the useEffect that contains loadTokens (and referenced functions loadTokensForChain, loadTokens, setAvailableTokens, setIsLoadingTokens) to [chainId] only.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 39: The package.json dependency for "zod" is pinned to v3 ("zod":
"^3.25.76"); update that version constraint to a v4 range (for example "zod":
"^4.3.6" or "^4.0.0") in package.json to allow zod v4, then run npm/yarn install
and your test suite; if any breaking changes surface, update usages of Zod APIs
(imports/usages of Zod functions/classes) to match v4 semantics.
In `@src/app/layout.tsx`:
- Line 149: package.json currently lists the unused dependency "geist"
(v^1.7.0); remove the "geist" entry from dependencies in package.json, reinstall
or run your package manager's install (npm/yarn/pnpm) to update node_modules and
the lockfile, and run a quick grep/IDE search for any remaining "geist" imports
or references (e.g., in layout.tsx, globals.css, tailwind.config.ts) to confirm
nothing else needs deleting.
In `@src/components/Forms/FormData.ts`:
- Around line 37-118: Extract all user-facing validation strings used in the Zod
validation block (the messages passed to ctx.addIssue for fields poolName,
baseTokenAddress, initialDeposit, priceFeedAddress, hebeswapPairAddress,
hebeswapQuoteToken, and any duplicates later in the file) into your i18n
resource files/constants and replace the literal strings with lookups; keep the
validation logic (ETH_ADDRESS_REGEX checks, Number parsing for initialDeposit,
and the conditional oracleType checks) intact and reference the i18n keys when
calling ctx.addIssue so copy is centralized and localizable.
- Around line 66-79: The validation currently uses Number.isNaN(deposit) which
lets non-finite numeric strings slip through; update the guard in the
initialDeposit validation to use Number.isFinite by checking
!Number.isFinite(deposit) so that NaN and non-finite values (e.g., Infinity) are
rejected before the negative check; keep the existing ctx.addIssue calls (path:
["initialDeposit"], code: z.ZodIssueCode.custom) and only replace the first
condition that references deposit.
- Around line 157-162: FeeStepSchema currently allows trimmed empty strings
because Number("") -> 0 and the numeric check passes; update the schema for
mintFee, burnFee, creatorFee, and treasuryFee to reject empty strings and ensure
they are numeric and positive (or non-zero as required). Replace
z.string().trim() with either z.string().trim().nonempty().refine(s =>
Number.isFinite(Number(s)) && Number(s) > 0, { message: 'must be a positive
number' }) or use z.preprocess to coerce to number and validate with
z.number().finite().positive(), ensuring the schema (FeeStepSchema) enforces
non-empty, numeric, and >0 values for all four fee fields.
In `@src/components/ui/TokenImage.tsx`:
- Around line 39-47: Remove the redundant unoptimized prop from the Image JSX in
TokenImage (the <Image ... /> element that uses src, alt,
width={sizePixels[size]}, height={sizePixels[size]}, className and onError={()
=> setHasError(true)}). Edit the TokenImage component to delete the unoptimized
attribute so the Image relies on the global images.unoptimized setting in
next.config.mjs; leave the other props (src, alt, width/height, className,
onError and setHasError) unchanged.
In `@tsconfig.json`:
- Line 19: The tsconfig setting "incremental": false was added to avoid stale
type references but harms build performance; revert "incremental" back to true
in tsconfig and instead diagnose and fix the root cause by deleting any stale
.tsbuildinfo files, running a full clean build (tsc --build or rm .tsbuildinfo
then tsc), and verifying compilerOptions and path alias consistency (baseUrl,
paths, composite/project references) across tsconfig(s) so incremental
compilation can run reliably without producing stale caches.
---
Outside diff comments:
In `@src/components/Forms/Steps/PoolConfigurationStep.tsx`:
- Around line 43-63: The useEffect that defines loadTokens currently lists
formData.baseTokenAddress in its dependency array causing unnecessary reloads;
remove formData.baseTokenAddress from that dependency array so the effect only
depends on chainId, since loadTokensForChain(chainId) and setAvailableTokens
only need chainId, and the separate effect that watches
formData.baseTokenAddress (which calls setSelectedToken) will handle updating
the selection; update the dependency array on the useEffect that contains
loadTokens (and referenced functions loadTokensForChain, loadTokens,
setAvailableTokens, setIsLoadingTokens) to [chainId] only.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3a7681a2-0ef3-4877-b222-e52b85005c64
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
eslint.config.mjspackage.jsonsrc/app/layout.tsxsrc/app/portfolio/page.tsxsrc/components/Forms/CreateFatePool.tsxsrc/components/Forms/FormData.tssrc/components/Forms/Steps/PoolConfigurationStep.tsxsrc/components/ui/TokenImage.tsxsrc/lib/validation.tssrc/utils/wagmiConfig.tstailwind.config.tstsconfig.json
💤 Files with no reviewable changes (1)
- src/lib/validation.ts
### 1. Context Closes StabilityNexus#79. Replaced manual string-based error messages with a centralized i18n object (`validationMessages`) for the pool configuration validation step to prevent hardcoded duplication. Also updated Zod to version 4 to resolve validation bugs and enabled incremental compilation for faster TS builds. ### 2. Implementation Details - `src/components/Forms/FormData.ts`: Centralized all string validations using `msg` from `validationMessages`. Changed `Number.isNaN` to `Number.isFinite` for accurate validation. - `src/lib/i18n/validationMessages.ts`: Extracted and grouped all UI text constants. - `src/components/Forms/Steps/PoolConfigurationStep.tsx`: Fixed redundant dependencies in `useEffect` list. - `src/components/ui/TokenImage.tsx`: Removed redundant `unoptimized` flag on NextJS Image component. - `package.json` & `tsconfig.json`: Bumped `zod` dependency to `^4.0.0` to support schema rules and removed `geist` font, enabled `incremental` compiler switch. ### 3. Verification - Verified local syntax logic with Typescript linting passes. - Confirmed UI logic for new pool components remains regression-free.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Forms/FormData.ts`:
- Around line 159-188: The per-field finitude/negativity checks should be
removed from StepThreeFormDataSchema.superRefine so it only enforces the
aggregate totalFee >= 100 rule; consolidate all per-field validations into
FeeStepSchema (ensure mintFee, burnFee, creatorFee, treasuryFee each use
.string().trim().nonempty(...).refine(...) for finite and >0 with the intended
messages) and update error messages there as needed; after this change, leave
StepThreeFormDataSchema.superRefine to only compute the numeric sum of those
four fields and add an issue on "totalFee" when sum < 100; ensure this aligns
with the CreateFatePool.tsx error handler that preserves the first error per
field.
In `@src/components/ui/TokenImage.tsx`:
- Around line 39-43: TokenImage uses Next.js Image with external URLs (see the
Image usage in TokenImage component and src prop) but those hosts aren’t
allowlisted, and hostname validation still runs even with unoptimized:true;
update Next.js image config to add remotePatterns that allow
coin-images.coingecko.com and static.coingecko.com (include both http and https
patterns or a protocol wildcard and the hostnames) so the Image component can
load external token images without failing at config time.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9db8866f-55cd-49f7-a82f-812dcaaa9a62
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
package.jsonsrc/components/Forms/FormData.tssrc/components/Forms/Steps/PoolConfigurationStep.tsxsrc/components/ui/TokenImage.tsxsrc/lib/i18n/validationMessages.ts
Comment on lines
+7
to
+26
| export const FormDataSchema = z.object({ | ||
| poolName: z.string().trim(), | ||
| baseTokenAddress: z.string().trim(), | ||
| oracleType: z.enum(["chainlink", "hebeswap"]), | ||
| priceFeedAddress: z.string().trim(), | ||
| hebeswapPairAddress: z.string().trim(), | ||
| hebeswapQuoteToken: z.string().trim(), | ||
| bullCoinName: z.string().trim(), | ||
| bullCoinSymbol: z.string().trim(), | ||
| bearCoinName: z.string().trim(), | ||
| bearCoinSymbol: z.string().trim(), | ||
| creatorAddress: z.string().trim(), | ||
| mintFee: z.string().trim(), | ||
| burnFee: z.string().trim(), | ||
| creatorFee: z.string().trim(), | ||
| treasuryFee: z.string().trim(), | ||
| initialDeposit: z.string().trim(), | ||
| quoteTokenAddress: z.string().trim().optional(), | ||
| oracleDescription: z.string().trim().optional(), | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 'safeParse\(formData\)|result\.success|result\.error\.issues' src/components/Forms/CreateFatePool.tsx
printf '\n[result.data usage]\n'
rg -n 'result\.data' src/components/Forms/CreateFatePool.tsx || true
printf '\n[downstream raw formData consumers]\n'
rg -n -C2 'formData\.' src/components/Forms/Steps/ReviewStep.tsxRepository: StabilityNexus/Fate-EVM-Frontend
Length of output: 5229
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if FormData type is derived from schema
printf '[FormData type definition]\n'
rg -n 'type FormData|export type FormData' src/components/Forms/FormData.ts -A2
# Check form input handling for trim
printf '\n[Form input onChange handling]\n'
rg -n 'onChange|setFormData' src/components/Forms/CreateFatePool.tsx -B1 -A2 | head -50
# Check submission/creation function
printf '\n[Submission and pool creation]\n'
rg -n 'createPool|mutation|submit' src/components/Forms/CreateFatePool.tsx -B2 -A3 | head -60Repository: StabilityNexus/Fate-EVM-Frontend
Length of output: 1213
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the full createPool arguments to see which fields are trimmed
printf '[Full createPool call with all arguments]\n'
sed -n '470,510p' src/components/Forms/CreateFatePool.tsx
# Also check if there are any other submission paths
printf '\n[Other uses of formData beyond createPool]\n'
rg -n 'formData\.' src/components/Forms/CreateFatePool.tsx | grep -E '(submit|deploy|send|create)' || trueRepository: StabilityNexus/Fate-EVM-Frontend
Length of output: 1688
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check how fee fields are converted to BigInt
printf '[Fee field conversion logic]\n'
rg -n 'mintFee|burnFee|creatorFee|treasuryFee' src/components/Forms/CreateFatePool.tsx -B2 -A2 | grep -E '(Units|BigInt|parseFloat|parseUnits)' -A2 -B2
# Check if baseTokenAddress and other addresses are trimmed anywhere
printf '\n[Address field handling before submission]\n'
rg -n 'baseTokenAddress|priceFeedAddress|hebeswapPairAddress' src/components/Forms/CreateFatePool.tsx -B1 -A1 | head -40
# Check the full fee conversion section
printf '\n[Full fee conversion logic around submission]\n'
sed -n '450,480p' src/components/Forms/CreateFatePool.tsxRepository: StabilityNexus/Fate-EVM-Frontend
Length of output: 3842
🏁 Script executed:
#!/bin/bash
set -euoripgrep
# Check hebeswapPairAddress and hebeswapQuoteToken handling
printf '[Hebeswap address field handling]\n'
rg -n 'hebeswapPairAddress|hebeswapQuoteToken' src/components/Forms/CreateFatePool.tsx -B1 -A1 | head -40
# Check if priceFeedAddress is trimmed
printf '\n[priceFeedAddress trimming check]\n'
sed -n '249,260p' src/components/Forms/CreateFatePool.tsx
# Verify all args passed to createPool - check if any address/fee fields are missing trim
printf '\n[Complete createPool args]\n'
sed -n '473,486p' src/components/Forms/CreateFatePool.tsxRepository: StabilityNexus/Fate-EVM-Frontend
Length of output: 750
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check hebeswapPairAddress and hebeswapQuoteToken handling
printf '[Hebeswap address field handling]\n'
rg -n 'hebeswapPairAddress|hebeswapQuoteToken' src/components/Forms/CreateFatePool.tsx -B1 -A1 | head -40
# Check if priceFeedAddress is trimmed
printf '\n[priceFeedAddress trimming check]\n'
sed -n '249,260p' src/components/Forms/CreateFatePool.tsx
# Verify all args passed to createPool - check if any address/fee fields are missing trim
printf '\n[Complete createPool args]\n'
sed -n '473,486p' src/components/Forms/CreateFatePool.tsxRepository: StabilityNexus/Fate-EVM-Frontend
Length of output: 2250
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the full flow of fee field handling at submission
printf '[Fee field submission without trim]\n'
sed -n '230,240p' src/components/Forms/CreateFatePool.tsx
# Check quoteTokenAddress (optional) handling
printf '\n[quoteTokenAddress/initialDeposit handling]\n'
rg -n 'quoteTokenAddress|initialDeposit|initialDepositAmount' src/components/Forms/CreateFatePool.tsx -B1 -A1 | head -40
# Check oracleDescription field
printf '\n[oracleDescription usage]\n'
rg -n 'oracleDescription' src/components/Forms/CreateFatePool.tsx -B1 -A1 || echo "Not found in CreateFatePool"Repository: StabilityNexus/Fate-EVM-Frontend
Length of output: 2476
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if fees fields have any trimming elsewhere in submission flow
printf '[All fee field uses at submission]\n'
sed -n '233,237p' src/components/Forms/CreateFatePool.tsx
# Check if quoteTokenAddress field is optional or required
printf '\n[quoteTokenAddress optional field handling]\n'
rg -n 'quoteTokenAddress' src/components/Forms/FormData.ts -B2 -A2
# Summary: list all fields with their trim status at submission
printf '\n[Field trim status at createPool call]\n'
sed -n '473,486p' src/components/Forms/CreateFatePool.tsxRepository: StabilityNexus/Fate-EVM-Frontend
Length of output: 1618
Trim operations in FormDataSchema are ignored; only some fields are trimmed at submission, creating inconsistency.
The schema defines .trim() on all string fields, but safeParse() result is never merged back into state. More critically, at submission time only certain fields are manually trimmed (baseTokenAddress, poolName, coin names), while many are used untrimmed: fee fields (parsed via parseFloat(formData.mintFee)), address fields like priceFeedAddress and hebeswapPairAddress, and initialDeposit.
This inconsistency causes two problems:
- Review display reads untrimmed values — ReviewStep displays raw
formDatafields, showing any leading/trailing whitespace - Address validation is vulnerable — lines 250 and 324 compare untrimmed user input directly against
"0x0000...", so" 0x0000... "would pass the check
Either remove .trim() from the schema or apply it consistently at submission or on input change.
Update StepThreeFormDataSchema and FeeStepSchema to strictly check Number > 0 for fees, and validate aggregate totalFee >= 100 on 'totalFee' path. Add coin-images.coingecko.com and static.coingecko.com to Next.js images remotePatterns.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Forms/FormData.ts`:
- Around line 84-90: The Chainlink branch currently only checks presence of
priceFeedAddress; update the validation in the block that checks if
(data.oracleType === "chainlink" && !data.priceFeedAddress) to also validate the
address format using the existing ETH_ADDRESS_REGEX (same regex used for other
on-chain address fields) and call ctx.addIssue with path ["priceFeedAddress"]
and the same custom error behavior when the regex test fails; locate the
conditional around priceFeedAddress and replace the simple presence check with a
regex guard so malformed feed addresses are rejected before submission.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b985042c-ea8a-4e55-aa4a-11b0c0d47daf
📒 Files selected for processing (2)
next.config.mjssrc/components/Forms/FormData.ts
Update StepOneFormDataSchema to enforce ETH_ADDRESS_REGEX on the Chainlink priceFeedAddress field, rejecting malformed addresses.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/Forms/FormData.ts (1)
7-26:⚠️ Potential issue | 🟠 MajorSchema normalization is not guaranteed to be the canonical payload.
FormDataSchemanow normalizes with.trim(), but downstream flow still uses rawformDatain places instead of parsed output. That creates validation/display/submission drift (validated trimmed values vs submitted untrimmed values). Please make parsed data (safeParse(...).data) the single source of truth after step validation, or remove normalization transforms here to avoid false guarantees.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Forms/FormData.ts` around lines 7 - 26, FormDataSchema currently trims inputs but callers still use the raw formData, causing mismatch between validated/normalized values and what gets displayed/submitted; update the flow that handles validation to use FormDataSchema.safeParse(...)'s parsed output as the single source of truth (replace downstream uses of raw formData with the safeParse result.data after successful validation) wherever form submission/preview/display occurs, or alternatively remove the .trim() transforms on FormDataSchema to avoid implying normalization—look for usages of FormDataSchema, safeParse, formData, and places that read fields like poolName/priceFeedAddress/quoteTokenAddress to apply the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Forms/FormData.ts`:
- Around line 167-196: The four identical fee field definitions in FeeStepSchema
should be deduplicated by extracting a shared schema (e.g., feeStringSchema)
that encapsulates the chain .string().trim().nonempty({ message:
msg.invalidFeeValue }).refine((s) => Number.isFinite(Number(s)) && Number(s) >
0, { message: msg.positiveFeeRequired }), then reuse that schema for mintFee,
burnFee, creatorFee, and treasuryFee inside FeeStepSchema to ensure consistent
validation and messages across all fields.
---
Duplicate comments:
In `@src/components/Forms/FormData.ts`:
- Around line 7-26: FormDataSchema currently trims inputs but callers still use
the raw formData, causing mismatch between validated/normalized values and what
gets displayed/submitted; update the flow that handles validation to use
FormDataSchema.safeParse(...)'s parsed output as the single source of truth
(replace downstream uses of raw formData with the safeParse result.data after
successful validation) wherever form submission/preview/display occurs, or
alternatively remove the .trim() transforms on FormDataSchema to avoid implying
normalization—look for usages of FormDataSchema, safeParse, formData, and places
that read fields like poolName/priceFeedAddress/quoteTokenAddress to apply the
fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a9889c73-9420-4d34-9729-17552cba689d
📒 Files selected for processing (1)
src/components/Forms/FormData.ts
Comment on lines
+167
to
+196
| const FeeStepSchema = z.object({ | ||
| mintFee: z | ||
| .string() | ||
| .trim() | ||
| .nonempty({ message: msg.invalidFeeValue }) | ||
| .refine((s) => Number.isFinite(Number(s)) && Number(s) > 0, { | ||
| message: msg.positiveFeeRequired, | ||
| }), | ||
| burnFee: z | ||
| .string() | ||
| .trim() | ||
| .nonempty({ message: msg.invalidFeeValue }) | ||
| .refine((s) => Number.isFinite(Number(s)) && Number(s) > 0, { | ||
| message: msg.positiveFeeRequired, | ||
| }), | ||
| creatorFee: z | ||
| .string() | ||
| .trim() | ||
| .nonempty({ message: msg.invalidFeeValue }) | ||
| .refine((s) => Number.isFinite(Number(s)) && Number(s) > 0, { | ||
| message: msg.positiveFeeRequired, | ||
| }), | ||
| treasuryFee: z | ||
| .string() | ||
| .trim() | ||
| .nonempty({ message: msg.invalidFeeValue }) | ||
| .refine((s) => Number.isFinite(Number(s)) && Number(s) > 0, { | ||
| message: msg.positiveFeeRequired, | ||
| }), | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Deduplicate fee field schema to reduce drift risk.
The four fee fields repeat identical chains. Extract a shared fee-string schema and reuse it to keep future message/rule changes consistent.
♻️ Proposed refactor
+const PositiveFeeStringSchema = z
+ .string()
+ .trim()
+ .nonempty({ message: msg.invalidFeeValue })
+ .refine((s) => Number.isFinite(Number(s)) && Number(s) > 0, {
+ message: msg.positiveFeeRequired,
+ });
+
const FeeStepSchema = z.object({
- mintFee: z
- .string()
- .trim()
- .nonempty({ message: msg.invalidFeeValue })
- .refine((s) => Number.isFinite(Number(s)) && Number(s) > 0, {
- message: msg.positiveFeeRequired,
- }),
- burnFee: z
- .string()
- .trim()
- .nonempty({ message: msg.invalidFeeValue })
- .refine((s) => Number.isFinite(Number(s)) && Number(s) > 0, {
- message: msg.positiveFeeRequired,
- }),
- creatorFee: z
- .string()
- .trim()
- .nonempty({ message: msg.invalidFeeValue })
- .refine((s) => Number.isFinite(Number(s)) && Number(s) > 0, {
- message: msg.positiveFeeRequired,
- }),
- treasuryFee: z
- .string()
- .trim()
- .nonempty({ message: msg.invalidFeeValue })
- .refine((s) => Number.isFinite(Number(s)) && Number(s) > 0, {
- message: msg.positiveFeeRequired,
- }),
+ mintFee: PositiveFeeStringSchema,
+ burnFee: PositiveFeeStringSchema,
+ creatorFee: PositiveFeeStringSchema,
+ treasuryFee: PositiveFeeStringSchema,
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Forms/FormData.ts` around lines 167 - 196, The four identical
fee field definitions in FeeStepSchema should be deduplicated by extracting a
shared schema (e.g., feeStringSchema) that encapsulates the chain
.string().trim().nonempty({ message: msg.invalidFeeValue }).refine((s) =>
Number.isFinite(Number(s)) && Number(s) > 0, { message: msg.positiveFeeRequired
}), then reuse that schema for mintFee, burnFee, creatorFee, and treasuryFee
inside FeeStepSchema to ensure consistent validation and messages across all
fields.
kushbosamiya
changed the title
Author
|
close #79 , @blizet @Zahnentferner I have implemented and tested the zod validation could you please review and merge it at your convenience . |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@01-REVIEWS.md`:
- Line 28: The review flagged ETH_ADDRESS_REGEX in FormData.ts but the project
intentionally prefers smart-contract-side validation (see CreateFatePool) so
update the review comment and FormData.ts note to reflect that regex is used
only for lightweight form-format validation and checksum validation is delegated
to the smart contract; mention ETH_ADDRESS_REGEX, isAddress() (viem) as an
available alternative, but explicitly state the chosen architecture (smart
contract authoritative validation) and rationale to avoid recommending adding
viem.isAddress() in the frontend.
- Line 17: Replace the phrase "Three step schemas" with the hyphenated compound
"three-step schemas" wherever it appears as a modifier (e.g., the review line
that starts with "Three step schemas properly isolate...") so the grammar uses
the correct compound adjective form.
- Around line 37-48: The markdown contains fenced code blocks without blank
lines before and after which triggers lint warnings; update the markdown so each
fenced code block (the example importing isAddress, the poolName zod snippet,
and the closing example around the FormData note) has a blank line immediately
before the opening ``` and a blank line immediately after the closing ```,
ensure language tags (e.g., ```typescript) are present where appropriate, and
verify each block is properly opened and closed around the symbols mentioned
(isAddress import example, poolName zod snippet, and the FormData totalFee note)
so the linter no longer flags formatting errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bc2e6805-6094-456a-855a-df813ed3d79f
📒 Files selected for processing (1)
01-REVIEWS.md
|
|
||
| ### 2. Strengths | ||
|
|
||
| - **Good schema organization**: Three step schemas properly isolate validation concerns per form step (`FormData.ts:30-129`, `131-165`, `167-212`) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor grammar: use hyphen for compound modifier.
"Three step schemas" should be "three-step schemas" when used as a compound modifier before a noun.
-- **Good schema organization**: Three step schemas properly isolate validation concerns per form step (`FormData.ts:30-129`, `131-165`, `167-212`)
+- **Good schema organization**: Three-step schemas properly isolate validation concerns per form step (`FormData.ts:30-129`, `131-165`, `167-212`)🧰 Tools
🪛 LanguageTool
[grammar] ~17-~17: Use a hyphen to join words.
Context: ...s - Good schema organization: Three step schemas properly isolate validation...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@01-REVIEWS.md` at line 17, Replace the phrase "Three step schemas" with the
hyphenated compound "three-step schemas" wherever it appears as a modifier
(e.g., the review line that starts with "Three step schemas properly
isolate...") so the grammar uses the correct compound adjective form.
| | Severity | Issue | Location | | ||
| |----------|-------|----------| | ||
| | **HIGH** | Zod v4 specified but v3 likely installed (package-lock shows v3.22.4/3.25.x) — `safeParse` API differs significantly between versions | `package.json:38` | | ||
| | **HIGH** | ETH_ADDRESS_REGEX only checks format, not checksum — vulnerable to case-sensitive address bugs | `FormData.ts:4,53,91,107,121` | |
Contributor
There was a problem hiding this comment.
Reconsider the address validation recommendation.
The review flags ETH_ADDRESS_REGEX as a security gap and recommends using viem's isAddress() for checksum validation. However, this conflicts with the project's established preference for smart contract validation over duplicating validation logic in the frontend.
For basic format validation at the form level, a regex check is typically sufficient, especially when the smart contract will perform the authoritative validation. Adding checksum validation in the frontend may be redundant given this architecture. Based on learnings, in the CreateFatePool component, validation for assetId format is handled by the smart contract rather than frontend validation, and the user prefers smart contract validation over duplicating validation logic in the frontend.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@01-REVIEWS.md` at line 28, The review flagged ETH_ADDRESS_REGEX in
FormData.ts but the project intentionally prefers smart-contract-side validation
(see CreateFatePool) so update the review comment and FormData.ts note to
reflect that regex is used only for lightweight form-format validation and
checksum validation is delegated to the smart contract; mention
ETH_ADDRESS_REGEX, isAddress() (viem) as an available alternative, but
explicitly state the chosen architecture (smart contract authoritative
validation) and rationale to avoid recommending adding viem.isAddress() in the
frontend.
Comment on lines
+37
to
+48
| 1. **Fix Zod version**: Verify actual installed version and test `safeParse` compatibility, or downgrade to `"zod": "^3.24.0"` for consistency with installed v3 | ||
| 2. **Replace regex with viem**: Use viem's `isAddress` from `validation.ts` for robust checksum validation: | ||
| ```typescript | ||
| import { isAddress } from 'viem'; | ||
| // Replace ETH_ADDRESS_REGEX.test() with isAddress() | ||
| ``` | ||
| 3. **Add field length limits**: | ||
| ```typescript | ||
| poolName: z.string().trim().min(1).max(100), | ||
| ``` | ||
| 4. **Fix total fee logic**: Change `totalFee >= 100` to `totalFee > 100` at `FormData.ts:205` | ||
| 5. **Add upper bound validation** for initialDeposit (e.g., max 10^18 for ETH denomination) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Fix markdown formatting around code blocks.
The code examples are missing blank lines before and after the fenced code blocks, which violates markdown best practices and triggers linting warnings.
📝 Proposed formatting fixes
1. **Fix Zod version**: Verify actual installed version and test `safeParse` compatibility, or downgrade to `"zod": "^3.24.0"` for consistency with installed v3
2. **Replace regex with viem**: Use viem's `isAddress` from `validation.ts` for robust checksum validation:
+
```typescript
import { isAddress } from 'viem';
// Replace ETH_ADDRESS_REGEX.test() with isAddress()
```
+
3. **Add field length limits**:
+
```typescript
poolName: z.string().trim().min(1).max(100),
```
+
4. **Fix total fee logic**: Change `totalFee >= 100` to `totalFee > 100` at `FormData.ts:205`
5. **Add upper bound validation** for initialDeposit (e.g., max 10^18 for ETH denomination)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 1. **Fix Zod version**: Verify actual installed version and test `safeParse` compatibility, or downgrade to `"zod": "^3.24.0"` for consistency with installed v3 | |
| 2. **Replace regex with viem**: Use viem's `isAddress` from `validation.ts` for robust checksum validation: | |
| ```typescript | |
| import { isAddress } from 'viem'; | |
| // Replace ETH_ADDRESS_REGEX.test() with isAddress() | |
| ``` | |
| 3. **Add field length limits**: | |
| ```typescript | |
| poolName: z.string().trim().min(1).max(100), | |
| ``` | |
| 4. **Fix total fee logic**: Change `totalFee >= 100` to `totalFee > 100` at `FormData.ts:205` | |
| 5. **Add upper bound validation** for initialDeposit (e.g., max 10^18 for ETH denomination) | |
| 1. **Fix Zod version**: Verify actual installed version and test `safeParse` compatibility, or downgrade to `"zod": "^3.24.0"` for consistency with installed v3 | |
| 2. **Replace regex with viem**: Use viem's `isAddress` from `validation.ts` for robust checksum validation: |
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 39-39: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 42-42: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 44-44: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 46-46: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@01-REVIEWS.md` around lines 37 - 48, The markdown contains fenced code blocks
without blank lines before and after which triggers lint warnings; update the
markdown so each fenced code block (the example importing isAddress, the
poolName zod snippet, and the closing example around the FormData note) has a
blank line immediately before the opening ``` and a blank line immediately after
the closing ```, ensure language tags (e.g., ```typescript) are present where
appropriate, and verify each block is properly opened and closed around the
symbols mentioned (isAddress import example, poolName zod snippet, and the
FormData totalFee note) so the linter no longer flags formatting errors.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1. Context
Closes #79.
This PR consolidates Create Pool form validation into one runtime schema using Zod, replacing duplicated manual validation logic spread across the form component and
src/lib/validation.ts.2. Problem
Before this change, validation was fragmented:
src/components/Forms/CreateFatePool.tsx: manual regex +parseFloat/isNaNchecks insidevalidateCurrentStep()src/lib/validation.ts: overlapping pool-related validators (validatePoolCreation, fee/symbol/name checks)src/components/Forms/FormData.ts: compile-time interface only, no runtime schema enforcementImpact:
3. Current Behavior (before)
Reproduction:
CreateFatePool.tsxwhile similar rules also existed insrc/lib/validation.tsEvidence:
validateCurrentStep()(address regex + fee parsing)src/lib/validation.ts4. Proposed Solution
Implemented:
zodinpackage.jsonFormDataSchema+ step schemas insrc/components/Forms/FormData.tsStepOneFormDataSchemaStepTwoFormDataSchemaStepThreeFormDataSchemaFormDatatype from Zod schema (z.infer)validateCurrentStep()inCreateFatePool.tsxtosafeParseby stepsrc/lib/validation.ts5. Problems Encountered After Zod with its Step-by-step Approach
.next/types/...)incremental: falseintsconfig.jsonand rebuild cleanly.next/font/googleGeist fetch failure in build environments with restricted networksrc/app/layout.tsxand use existing global font stack.DEFAULT_PROJECT_IDcausing remote 403 behavior)src/utils/wagmiConfig.ts, disable WalletConnect fallback when project id is missing and only warn outside production.next linttoeslint .eslint.config.mjs(.next,out,next-env.d.ts)require("tailwindcss-animate")with ESM import intailwind.config.tssrc/app/portfolio/page.tsxto accept Recharts value unions safely<img>in token avatarnext/imageinsrc/components/ui/TokenImage.tsx.6. Implementation Details (files)
src/components/Forms/FormData.ts: Zod schema + step schemas + inferred form typesrc/components/Forms/CreateFatePool.tsx: step validation viasafeParsesrc/lib/validation.ts: removed overlapping pool validation utilitiespackage.json,package-lock.json: explicit zod dependency + dependency updatestsconfig.json:incremental: falsesrc/app/layout.tsx: removenext/font/googleGeist dependencysrc/utils/wagmiConfig.ts: safe missing-project-id fallbackeslint.config.mjs,tailwind.config.ts,src/components/ui/TokenImage.tsx,src/components/Forms/Steps/PoolConfigurationStep.tsx,src/app/portfolio/page.tsx: lint/type hardening7. Verification
Executed locally during this branch work:
npm run lintnpm run buildResult:
8. Acceptance Criteria
safeParseSummary by CodeRabbit
New Features
Bug Fixes
Style
Chores
Breaking Changes