feat:resolved Multi-Chain Compatibility and Network Management#19
feat:resolved Multi-Chain Compatibility and Network Management#19dark-Invincible wants to merge 1 commit into
Conversation
WalkthroughThis PR introduces network management infrastructure for multi-chain support, including three new TypeScript utilities for transaction validation and formatting, multi-network configuration and switching, a React hook for network state management, configuration enhancements with JSDoc documentation, a comprehensive implementation guide and documentation suite, and updates to the docs routing system. Changes
Sequence DiagramsequenceDiagram
participant Component as React Component
participant Hook as useNetwork Hook
participant Helpers as networkHelpers
participant Wagmi as wagmi
participant Blockchain as Blockchain
Component->>Hook: useNetwork()
Hook->>Wagmi: useAccount(), useSwitchChain()
Wagmi-->>Hook: current chainId, switchChain fn
rect rgb(240, 248, 255)
Note over Hook: Validate Current Chain
Hook->>Helpers: isSupportedChain(chainId)
Helpers-->>Hook: isSupported: boolean
end
alt Chain is Unsupported
Hook->>Helpers: getUnsupportedNetworkMessage(chainId)
Helpers-->>Hook: error message
else Chain is Supported
Hook->>Helpers: getNetworkName(chainId)
Helpers-->>Hook: networkName
end
Hook-->>Component: networkStatus { chainId, isSupported, networkName, error }
rect rgb(240, 255, 240)
Note over Component,Hook: User Initiates Chain Switch
Component->>Hook: switchToChainId(targetChainId)
Hook->>Helpers: needsChainSwitch(current, target)
Helpers-->>Hook: switchNeeded: boolean
alt Switch Required
Hook->>Wagmi: switchChain(targetChainId)
Wagmi->>Blockchain: initiate chain switch
Blockchain-->>Wagmi: transaction confirmation
Wagmi-->>Hook: success or error
end
end
Hook-->>Component: updated networkStatus + switchToChain methods
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (11)
README.md (1)
172-172: Documentation link could be clearer.The current format links to the source file rather than providing a useful documentation link. Consider linking to the actual deployed docs route or external documentation instead.
-- **Documentation**: [/src/app/docs/page.tsx](src/app/docs/page.tsx) — local docs route at `/docs` +- **Documentation**: [View Docs](/docs) — available at `/docs` when running locallysrc/app/docs/page.tsx (1)
1-1: Consider removing the"use client"directive.This component renders only static content with
Linkcomponents and has no client-side interactivity (no hooks, event handlers, or browser APIs). In Next.js 15,Linkworks in Server Components, so this page could benefit from being a Server Component for better performance and smaller client bundle.-"use client"; - import Link from "next/link";MASTER_CHECKLIST.md (1)
86-91: Add blank lines around the table for markdown compatibility.Some markdown parsers may not render the table correctly without surrounding blank lines.
### Code Quality Metrics + | Metric | Before | After | Target | |--------|--------|-------|--------| | JSDoc Coverage | ~1% | 75%+ | ✅ Pass | | ESLint Errors | TBD | 0 | ✅ Pass | | TypeScript Errors | TBD | 0 | ✅ Pass | | Test Coverage | TBD | TBD | Monitor | + ### Feature Completenesssrc/hooks/useNetwork.ts (1)
9-17: Remove unused importgetNetworkInfo.
getNetworkInfois imported but never used in this file.import { isSupportedChain, - getNetworkInfo, getNetworkName, getUnsupportedNetworkMessage, needsChainSwitch, getSupportedChainIds, type SupportedChainId, } from '@/utils/networkHelpers';src/utils/transactionHelpers.ts (2)
14-17: Address validation is too weak for prod flows (no checksum / zero-address guard).
Regex-only checks miss EIP-55 checksum validation and allow the zero address, which is commonly an invalid receiver/contract address in UX flows. Consider usingviem’sisAddress()/getAddress()(checksum + normalization) and explicitly rejecting0x000...000where appropriate.-import { parseUnits, formatUnits } from 'viem'; +import { parseUnits, formatUnits, isAddress, getAddress, zeroAddress } from 'viem'; export const isValidAddress = (address: string): boolean => { if (!address) return false; - return /^0x[a-fA-F0-9]{40}$/.test(address); + if (!isAddress(address)) return false; + return getAddress(address) !== zeroAddress; };
140-147:parseAmountis a good, safe entry point—consider reusing it internally.
To keep behavior consistent (decimals, validation semantics),validateTransactionParamsshould ideally callparseAmount(...)rather thanparseUnitsdirectly.IMPLEMENTATION_GUIDE.md (1)
29-35: Avoid recommending global/* eslint-disable */even as a “bad example” without a clear “DO” alternative nearby.
You do include a good alternative later; consider tightening the section so readers can’t cargo-cult the bad pattern.Also applies to: 203-211
src/utils/networkHelpers.ts (3)
82-85:isSupportedChaincheck should avoid falsy guard and be explicit aboutundefined.
if (!chainId)treats0the same asundefined(fine), but being explicit prevents accidental behavior changes and reads clearer.export const isSupportedChain = (chainId: number | undefined): chainId is SupportedChainId => { - if (!chainId) return false; + if (chainId === undefined) return false; return chainId in SUPPORTED_NETWORKS; };Also applies to: 92-95
153-155:needsChainSwitchshould also validate target support (and optionally current support).
As written, switching from unsupported→unsupported returnstruewithout context; callers may prefer “cannot switch to target (unsupported)”.
162-167:getUnsupportedNetworkMessageignores the passedchainId; consider including the actual id/name.
This makes error reports actionable (“Connected to chain 42161 which is unsupported…”).CODE_ANALYSIS_SUMMARY.md (1)
3-3: Minor style nit: some guides prefer a comma after the year in “December 13, 2025”.
Not required, but this is what LanguageTool flagged.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
CODE_ANALYSIS_SUMMARY.md(1 hunks)IMPLEMENTATION_GUIDE.md(1 hunks)MASTER_CHECKLIST.md(1 hunks)PROJECT_OVERVIEW.txt(1 hunks)QUICK_START.md(1 hunks)README.md(1 hunks)src/app/docs/page.tsx(1 hunks)src/app/page.tsx(1 hunks)src/hooks/useNetwork.ts(1 hunks)src/utils/networkHelpers.ts(1 hunks)src/utils/transactionHelpers.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/hooks/useNetwork.ts (1)
src/utils/networkHelpers.ts (6)
isSupportedChain(82-85)getUnsupportedNetworkMessage(162-167)SupportedChainId(75-75)getNetworkName(102-104)needsChainSwitch(153-155)getSupportedChainIds(143-145)
🪛 LanguageTool
CODE_ANALYSIS_SUMMARY.md
[style] ~3-~3: Some style guides suggest that commas should set off the year in a month-day-year date.
Context: ...alysis Analysis Date: December 13, 2025 Project: Djed-Solidity-ERC20BaseC...
(MISSING_COMMA_AFTER_YEAR)
PROJECT_OVERVIEW.txt
[style] ~226-~226: Try moving the adverb to make the sentence clearer.
Context: ...════════════════════════════════ ✓ How to properly document React components ✓ How to validate user inputs securely ...
(SPLIT_INFINITIVE)
🪛 markdownlint-cli2 (0.18.1)
CODE_ANALYSIS_SUMMARY.md
234-234: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
MASTER_CHECKLIST.md
24-24: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
39-39: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
58-58: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
68-68: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
86-86: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
156-156: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
309-309: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
336-336: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (5)
PROJECT_OVERVIEW.txt (1)
1-333: Documentation structure looks good.The PROJECT_OVERVIEW.txt provides a comprehensive project briefing with clear sections on deliverables, implementation phases, and quality metrics. The ASCII formatting makes it easy to scan.
QUICK_START.md (1)
1-272: Quick start guide is well-organized.The documentation provides clear, actionable steps for implementing the new utilities with helpful code examples. The structure (What Was Fixed → New Files → Quick Implementation → Key Patterns) makes it easy to follow.
src/app/docs/page.tsx (1)
5-49: Clean component implementation.The DocsPage component is well-structured with clear sections, proper semantic HTML, and consistent styling. The Quick Links provide useful navigation to key routes.
MASTER_CHECKLIST.md (1)
1-345: Comprehensive implementation checklist.The MASTER_CHECKLIST.md provides excellent tracking for implementation progress, quality assurance, and common issues. The structure with phases, dependencies, and success indicators is well thought out.
src/hooks/useNetwork.ts (1)
68-137: Well-structured network management hook.The
useNetworkhook provides a clean API for network detection and switching. Good use ofuseCallbackfor memoization, proper effect cleanup, and theNetworkStatusobject encapsulates network state effectively.
| | Issue | Severity | Files Affected | Status | | ||
| |-------|----------|------------------|--------| | ||
| | No JSDoc Documentation | 🔴 CRITICAL | 4 files | FIXED | | ||
| | Global ESLint Disables | 🟡 MEDIUM | trade/page.tsx | FIXED | | ||
| | Missing Input Validation | 🔴 CRITICAL | Multiple | FIXED | | ||
| | No Network Switching | 🔴 CRITICAL | wagmiConfig | FIXED | | ||
| | Performance Issues | 🟡 MEDIUM | Trade page | FIXED | | ||
| | Poor Error Messages | 🟡 MEDIUM | Trade page | FIXED | | ||
| | Missing Accessibility | 🟡 MEDIUM | UI Components | FIXED | | ||
|
|
There was a problem hiding this comment.
The “Status: FIXED” table is risky unless it’s guaranteed by this PR’s actual code changes.
If these are intended as goals or “recommended fixes”, label them as such (e.g., “Planned” / “In progress”) to prevent reviewers from assuming the codebase already changed.
🤖 Prompt for AI Agents
In CODE_ANALYSIS_SUMMARY.md around lines 13 to 22, the table marks items as
"FIXED" which can be misleading unless the PR actually made those changes;
update the table to reflect intent (e.g., change "Status" values to "Planned",
"In progress", or "Verified" and only mark "Verified"/"Fixed" when confirmed by
code changes in this PR), add a short note or column indicating how the status
was determined (PR number or verification step), and ensure the wording above
the table clarifies these are recommended fixes or goals rather than guaranteed
changes.
| **Usage Example:** | ||
| ```typescript | ||
| import { validateTransactionParams, formatBalance } from '@/utils/transactionHelpers'; | ||
|
|
||
| // Validate before transaction | ||
| const { isValid, errors } = validateTransactionParams({ | ||
| amount: userInput, | ||
| receiver: walletAddress, | ||
| balance: userBalance, | ||
| allowance: userAllowance, | ||
| }); | ||
|
|
||
| if (!isValid) { | ||
| setErrors(errors); | ||
| return; | ||
| } | ||
|
|
||
| // Format for display | ||
| const displayBalance = formatBalance(balance, 18, 2); // "1,234.56" | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Code fences should specify a language for markdownlint compliance.
Static analysis flagged MD040; add ts/tsx/shell as appropriate.
Also applies to: 102-114, 145-165
🤖 Prompt for AI Agents
In CODE_ANALYSIS_SUMMARY.md around lines 49 to 69 (and also for the other
flagged ranges 102-114 and 145-165), the markdown code fences are missing
language specifiers which triggers MD040; update each fence to include the
appropriate language hint (e.g., ```ts or ```tsx for TypeScript snippets,
```shell for command examples) so the fenced blocks start with the correct
language token and the file complies with markdownlint MD040.
| ## Executive Summary | ||
|
|
||
| Your Djed Protocol Web UI has been analyzed for CodeRabbit compliance and future-proofing. This document outlines all issues found and the recommended fixes. | ||
|
|
There was a problem hiding this comment.
Doc reads like all issues are already fixed, but many sections describe “changes needed”.
This is likely to mislead reviewers/users. Suggest rewording to clearly separate “implemented in this PR” vs “recommended follow-ups / next steps”.
Also applies to: 116-175, 268-339
🤖 Prompt for AI Agents
In IMPLEMENTATION_GUIDE.md around lines 3 to 6 (and similarly at 116-175 and
268-339), the Executive Summary and other sections read as if all issues are
already fixed; update language to clearly separate items implemented in this PR
from recommended follow-ups: add two distinct subsections titled "Changes in
this PR" and "Recommended Next Steps/Follow-ups", move any completed fixes into
the first subsection with brief bullet summaries and PR references, and move
outstanding action items into the follow-ups subsection with clear owners,
priorities, and suggested timelines; ensure phrasing uses present/past tense for
implemented items and modal/imperative language for recommendations.
| ``` | ||
| src/ | ||
| ├── app/ | ||
| │ ├── trade/page.tsx (UPDATED: Add JSDoc, validation, error handling) | ||
| │ ├── dashboard/page.tsx (Future: Add JSDoc) | ||
| │ └── ... | ||
| ├── components/ | ||
| │ ├── layout/ | ||
| │ │ ├── Navbar.tsx (Future: Add JSDoc, network selector) | ||
| │ │ └── ... | ||
| │ └── ui/ | ||
| │ └── ... | ||
| ├── context/ | ||
| │ └── walletProvider.tsx (UPDATED: Add comprehensive JSDoc) | ||
| └── utils/ | ||
| ├── addresses.ts (UPDATED: Add module JSDoc) | ||
| ├── wagmiConfig.ts (UPDATED: Add JSDoc, validation) | ||
| ├── transactionHelpers.ts (✨ NEW: Transaction utilities) | ||
| ├── networkHelpers.ts (✨ NEW: Network management) | ||
| ├── chainConfig.ts (Future: Document) | ||
| └── abi/ | ||
| ├── Djed.json | ||
| ├── Coin.json | ||
| └── IOracle.json | ||
| ``` |
There was a problem hiding this comment.
File tree includes future/updated files not present in this review set—consider marking them explicitly as “planned”.
Helps avoid “doc drift” if follow-up PRs don’t land.
🤖 Prompt for AI Agents
In IMPLEMENTATION_GUIDE.md around lines 525 to 549, the file tree lists entries
marked as “UPDATED” or “Future” that are not present in the current review set;
update the document to explicitly mark non-included files as “planned” (or
similar) and add a short note explaining that these entries are planned for
future PRs and may not exist in this branch to prevent doc drift — replace or
augment the current annotations (UPDATED/Future) with a clear “PLANNED” tag and
one-line guidance about updating the guide when those files land.
| <Link href="/docs" className="px-8 py-4 rounded-xl border border-white/30 hover:bg-white/10 transition-all duration-300 text-lg font-semibold backdrop-blur-sm"> | ||
| View Documentation | ||
| </button> | ||
| </Link> |
There was a problem hiding this comment.
Missing Link import from next/link.
The Link component is used on line 319 but is not imported. The imports section (lines 3-26) does not include import Link from 'next/link'.
Add the missing import at the top of the file:
'use client';
import { useState, useEffect } from 'react';
+import Link from 'next/link';
import {
ArrowRight, 📝 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.
| <Link href="/docs" className="px-8 py-4 rounded-xl border border-white/30 hover:bg-white/10 transition-all duration-300 text-lg font-semibold backdrop-blur-sm"> | |
| View Documentation | |
| </button> | |
| </Link> | |
| 'use client'; | |
| import { useState, useEffect } from 'react'; | |
| import Link from 'next/link'; | |
| import { | |
| ArrowRight, |
🤖 Prompt for AI Agents
In src/app/page.tsx around lines 319 to 321, the Link component is used but not
imported; add a top-level import statement for Link from 'next/link' (e.g., add
"import Link from 'next/link'" to the existing imports block at the top of the
file, within lines 3-26) so the component is available where it's used.
| try { | ||
| switchChain({ chainId: targetChainId as SupportedChainId }); | ||
| } catch (err) { | ||
| const message = err instanceof Error ? err.message : 'Failed to switch chain'; | ||
| setError(message); | ||
| } | ||
| }, | ||
| [switchChain] | ||
| ); |
There was a problem hiding this comment.
Add await to switchChain call to properly catch errors.
The switchChain function from wagmi returns a Promise, but it's not awaited. This means errors thrown during chain switching won't be caught by the try-catch block, leaving the error state stale.
try {
- switchChain({ chainId: targetChainId as SupportedChainId });
+ await switchChain({ chainId: targetChainId as SupportedChainId });
} catch (err) {
const message = err instanceof Error ? err.message : 'Failed to switch chain';
setError(message);
}📝 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.
| try { | |
| switchChain({ chainId: targetChainId as SupportedChainId }); | |
| } catch (err) { | |
| const message = err instanceof Error ? err.message : 'Failed to switch chain'; | |
| setError(message); | |
| } | |
| }, | |
| [switchChain] | |
| ); | |
| try { | |
| await switchChain({ chainId: targetChainId as SupportedChainId }); | |
| } catch (err) { | |
| const message = err instanceof Error ? err.message : 'Failed to switch chain'; | |
| setError(message); | |
| } | |
| }, | |
| [switchChain] | |
| ); |
🤖 Prompt for AI Agents
In src/hooks/useNetwork.ts around lines 99 to 107, the switchChain call returns
a Promise but isn't awaited, so errors escape the try-catch; update the code to
await switchChain (i.e., await switchChain({ chainId: targetChainId as
SupportedChainId })) and mark the enclosing callback/function as async so the
try-catch can properly catch and setError on failures.
| export const SUPPORTED_NETWORKS = { | ||
| 1: { | ||
| name: 'Ethereum Mainnet', | ||
| symbol: 'ETH', | ||
| rpcUrl: 'https://eth.rpc.blxrbdn.com', | ||
| blockExplorer: 'https://etherscan.io', | ||
| icon: '🔵', | ||
| isTestnet: false, | ||
| }, | ||
| 11155111: { | ||
| name: 'Sepolia Testnet', | ||
| symbol: 'ETH', | ||
| rpcUrl: 'https://sepolia.infura.io/v3/', | ||
| blockExplorer: 'https://sepolia.etherscan.io', | ||
| icon: '🧪', | ||
| isTestnet: true, | ||
| }, | ||
| 137: { | ||
| name: 'Polygon (Matic)', | ||
| symbol: 'MATIC', | ||
| rpcUrl: 'https://polygon-rpc.com', | ||
| blockExplorer: 'https://polygonscan.com', | ||
| icon: '🟣', | ||
| isTestnet: false, | ||
| }, | ||
| 56: { | ||
| name: 'Binance Smart Chain', | ||
| symbol: 'BNB', | ||
| rpcUrl: 'https://bsc-dataseed.binance.org', | ||
| blockExplorer: 'https://bscscan.com', | ||
| icon: '🟡', | ||
| isTestnet: false, | ||
| }, | ||
| 8453: { | ||
| name: 'Base', | ||
| symbol: 'ETH', | ||
| rpcUrl: 'https://mainnet.base.org', | ||
| blockExplorer: 'https://basescan.org', | ||
| icon: '🔵', | ||
| isTestnet: false, | ||
| }, | ||
| 61: { | ||
| name: 'Ethereum Classic', | ||
| symbol: 'ETC', | ||
| rpcUrl: 'https://etc.rivet.link', | ||
| blockExplorer: 'https://blockscout.com/etc/mainnet', | ||
| icon: '♻️', | ||
| isTestnet: false, | ||
| }, | ||
| 2001: { | ||
| name: 'Milkomeda', | ||
| symbol: 'MALK', | ||
| rpcUrl: 'https://mainnet-jsonrpc.milkomeda.com', | ||
| blockExplorer: 'https://mainnet-explorer.milkomeda.com', | ||
| icon: '🟢', | ||
| isTestnet: false, | ||
| }, | ||
| } as const; |
There was a problem hiding this comment.
Hardcoded RPC URLs are brittle; Sepolia endpoint is unusable as written.
https://sepolia.infura.io/v3/ won’t work without a project id, and hardcoding public RPCs risks rate limits/outages (also a stated objective: “endpoint reliability”). Consider moving RPC URLs to env/config, allowing multiple per chain with fallback, and validating at startup.
| export const isValidAmount = (amount: string, decimals: number = 18): boolean => { | ||
| if (!amount || amount === '0' || isNaN(Number(amount))) return false; | ||
| try { | ||
| parseUnits(amount, decimals); | ||
| return true; | ||
| } catch { | ||
| return false; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the import statement for parseUnits
cd src/utils && grep -r "parseUnits" transactionHelpers.tsRepository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 272
🏁 Script executed:
# Look at the full file context
cat -n src/utils/transactionHelpers.ts | head -50Repository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 2162
🏁 Script executed:
# Search for tests related to isValidAmount
find . -type f \( -name "*.test.ts" -o -name "*.spec.ts" \) | xargs grep -l "isValidAmount" 2>/dev/null || echo "No test files found"Repository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 109
🌐 Web query:
parseUnits behavior with zero and negative values ethers.js
💡 Result:
Short answer
- parseUnits("0", ...) returns a BigNumber zero. [1][2]
- parseUnits accepts negative decimal strings (e.g. parseEther("-0.5") → BigNumber negative wei) and returns a negative BigNumber. [1][2]
- parseUnits will throw errors for invalid inputs: fractional digits > unit decimals, numeric overflow (too large), or other numeric faults. [3][4]
Notes / gotchas
- parseUnits parses strings; passing non-string JS numbers can lose precision — prefer strings. [1][2]
- BigNumber returned supports negative values for arithmetic, but some BigNumber operations (bitwise, negative pow/shift) are unsupported. [3]
- Very large parsed values can overflow (RangeError/NUMERIC_FAULT). See GitHub issue for examples. [4]
Sources
[1] ethers.js v5 display-logic / parseUnits & parseEther examples.
[2] ethers.js docs (single-page) — parseUnits/parseEther examples.
[3] ethers.js errors docs (NUMERIC_FAULT: fractional component, underflow, unsupported negative ops).
[4] GitHub issue showing parseUnits overflow behavior.
🏁 Script executed:
# Search for usages of isValidAmount
rg "isValidAmount" --type ts --type tsx -B 2 -A 2Repository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 117
🏁 Script executed:
# Look for viem documentation or check what version is being used
cat -n package.json | grep -A 5 -B 5 "viem"Repository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 440
🏁 Script executed:
# Check if there are any integration tests or examples that show expected behavior
find . -type f \( -name "*.ts" -o -name "*.tsx" \) | xargs grep -l "isValidAmount\|parseUnits" | head -10Repository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 174
🏁 Script executed:
# Check usages in create page
cat -n src/app/create/page.tsx | grep -B 5 -A 5 "isValidAmount"Repository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 72
🏁 Script executed:
# Check usages in trade page
cat -n src/app/trade/page.tsx | grep -B 5 -A 5 "isValidAmount"Repository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 72
🌐 Web query:
viem parseUnits negative values zero behavior version 2
💡 Result:
Short answer: In viem v2 parseUnits expects a non‑negative decimal string and returns a bigint; parseUnits("0", decimals) → 0n. The official viem docs show parseUnits takes a string and returns a bigint but do not document support for negative inputs, so negative values should be validated/avoided in your code (they will not meaningfully produce a negative bigint like some BigNumber libraries — treat negative input as invalid). See viem parseUnits doc and the ethers migration notes for behavior expectations. [1][2]
Sources:
- viem parseUnits docs. [1]
- viem ethers → viem migration (parseUnits examples / expectations). [2]
If you want, I can run a quick code example for the exact viem version you use to show the thrown error or output.
🏁 Script executed:
# Broader search for isValidAmount without grep context
rg "isValidAmount" --type tsRepository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 380
🏁 Script executed:
# Check if the file paths actually have the function being used
fd "page.tsx" | head -5Repository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 204
🏁 Script executed:
# Test viem parseUnits behavior with negative and zero values
cat > /tmp/test_parseunits.js << 'EOF'
// Quick test to understand viem parseUnits behavior
const testCases = ["0", "0.0", "00", "-1", "-0.5", "1.5"];
console.log("Testing parseUnits behavior:");
testCases.forEach(val => {
console.log(` Input: "${val}"`);
console.log(` typeof val: ${typeof val}`);
console.log(` val === "0": ${val === "0"}`);
console.log(` Number(val): ${Number(val)}`);
console.log(` Number(val) <= 0: ${Number(val) <= 0}`);
console.log();
});
EOF
node /tmp/test_parseunits.jsRepository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 769
isValidAmount accepts zero-ish and negative values.
The check amount === '0' only catches the exact string "0" and misses "0.0", "00", and similar variants. Additionally, negative values like "-1" and "-0.5" are not rejected by the current logic and will be accepted by parseUnits. Since transaction amounts must be positive, replace the brittle string comparison with an explicit numeric check.
export const isValidAmount = (amount: string, decimals: number = 18): boolean => {
- if (!amount || amount === '0' || isNaN(Number(amount))) return false;
+ if (!amount || isNaN(Number(amount))) return false;
+ if (Number(amount) <= 0) return false;
try {
parseUnits(amount, decimals);
return true;
} catch {
return false;
}
};📝 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.
| export const isValidAmount = (amount: string, decimals: number = 18): boolean => { | |
| if (!amount || amount === '0' || isNaN(Number(amount))) return false; | |
| try { | |
| parseUnits(amount, decimals); | |
| return true; | |
| } catch { | |
| return false; | |
| } | |
| }; | |
| export const isValidAmount = (amount: string, decimals: number = 18): boolean => { | |
| if (!amount || isNaN(Number(amount))) return false; | |
| if (Number(amount) <= 0) return false; | |
| try { | |
| parseUnits(amount, decimals); | |
| return true; | |
| } catch { | |
| return false; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In src/utils/transactionHelpers.ts around lines 25 to 33, the current amount
check uses a brittle string comparison (amount === '0') and fails to reject
zero-like strings and negatives; replace that with an explicit numeric check:
trim the input, ensure parseFloat(amount) is a finite number greater than 0
(reject NaN, <= 0, and negative values), then keep the existing parseUnits
validation inside the try/catch to verify format with the given decimals; return
true only if both the numeric positivity check and parseUnits succeed, otherwise
return false.
| export const formatBalance = ( | ||
| balance: bigint | undefined, | ||
| decimals: number = 18, | ||
| decimalPlaces: number = 2 | ||
| ): string => { | ||
| if (!balance) return '0'; | ||
| const formatted = formatUnits(balance, decimals); | ||
| return parseFloat(formatted).toLocaleString('en-US', { | ||
| minimumFractionDigits: 0, | ||
| maximumFractionDigits: decimalPlaces, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
formatBalance / formatPrice lose precision for large values (parseFloat on formatUnits).
parseFloat will round large token balances and can silently misrepresent values; this is risky in a financial UI. Prefer string-based formatting (split on . and truncate) instead of floating point.
export const formatBalance = (
balance: bigint | undefined,
decimals: number = 18,
decimalPlaces: number = 2
): string => {
if (!balance) return '0';
- const formatted = formatUnits(balance, decimals);
- return parseFloat(formatted).toLocaleString('en-US', {
- minimumFractionDigits: 0,
- maximumFractionDigits: decimalPlaces,
- });
+ const formatted = formatUnits(balance, decimals); // string
+ const [intPart, fracPart = ''] = formatted.split('.');
+ const trimmedFrac = fracPart.slice(0, decimalPlaces).replace(/0+$/, '');
+ const withCommas = BigInt(intPart || '0').toLocaleString('en-US');
+ return trimmedFrac ? `${withCommas}.${trimmedFrac}` : withCommas;
}; export const formatPrice = (price: bigint | undefined): string => {
if (!price) return '$0.00';
- const formatted = formatUnits(price, 18);
- const value = parseFloat(formatted);
- return `$${value.toLocaleString('en-US', {
- minimumFractionDigits: 2,
- maximumFractionDigits: 6,
- })}`;
+ const formatted = formatUnits(price, 18);
+ const [intPart, fracPart = ''] = formatted.split('.');
+ const frac = fracPart.padEnd(2, '0').slice(0, 6);
+ const dollars = BigInt(intPart || '0').toLocaleString('en-US');
+ const shownFrac = frac.slice(0, Math.max(2, frac.replace(/0+$/, '').length));
+ return `$${dollars}.${shownFrac}`;
};Also applies to: 61-69
🤖 Prompt for AI Agents
In src/utils/transactionHelpers.ts around lines 42 to 53 (and similarly lines 61
to 69), the implementation uses parseFloat(formatUnits(...)) which can lose
precision for very large token balances; replace floating-point parsing with
string-based handling: obtain the decimal string from formatUnits, split on the
decimal point, pad/truncate the fractional part to the requested decimalPlaces
without converting to Number, and assemble the integer and fractional parts
(inserting commas into the integer part using string-based grouping) so the
returned value preserves exact precision and only truncates (not rounds) the
fractional portion.
| export const validateTransactionParams = (params: { | ||
| amount?: string; | ||
| receiver?: string; | ||
| fromAddress?: string; | ||
| allowance?: bigint; | ||
| balance?: bigint; | ||
| }): { isValid: boolean; errors: string[] } => { | ||
| const errors: string[] = []; | ||
|
|
||
| if (!params.amount || !isValidAmount(params.amount)) { | ||
| errors.push('Invalid amount'); | ||
| } | ||
|
|
||
| if (params.receiver && !isValidAddress(params.receiver)) { | ||
| errors.push('Invalid receiver address'); | ||
| } | ||
|
|
||
| if (params.fromAddress && !isValidAddress(params.fromAddress)) { | ||
| errors.push('Invalid from address'); | ||
| } | ||
|
|
||
| if (params.allowance !== undefined && params.balance !== undefined) { | ||
| const amountBig = parseUnits(params.amount || '0', 18); | ||
| if (params.balance < amountBig) { | ||
| errors.push('Insufficient balance'); | ||
| } | ||
| if (params.allowance < amountBig) { | ||
| errors.push('Insufficient allowance'); | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| isValid: errors.length === 0, | ||
| errors, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "transactionHelpers.ts" -type fRepository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 123
🏁 Script executed:
cat -n src/utils/transactionHelpers.ts | sed -n '90,135p'Repository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 1618
🏁 Script executed:
wc -l src/utils/transactionHelpers.tsRepository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 125
🏁 Script executed:
cat -n src/utils/transactionHelpers.tsRepository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 5773
🏁 Script executed:
find src/utils -type f 2>/dev/null | head -20Repository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 457
🏁 Script executed:
git ls-files | grep -i transactionRepository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 121
🏁 Script executed:
ls -la && find . -maxdepth 2 -type d | head -20Repository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 1805
🏁 Script executed:
if [ -d "src" ]; then find src -type f -name "*.ts" | head -30; else echo "src not found"; git ls-files | grep "\.ts$" | head -30; fiRepository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 450
🏁 Script executed:
git ls-files | xargs grep -l "validateTransactionParams\|parseUnits" 2>/dev/null | head -20Repository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 253
🏁 Script executed:
git ls-files | grep -E "(helper|util|transaction)" | head -30Repository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 474
🏁 Script executed:
git ls-files --others --exclude-standard && git ls-files | wc -l && echo "---" && git ls-files | head -50Repository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 1415
🏁 Script executed:
rg "validateTransactionParams" --type ts --type tsx -B 2 -A 2Repository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 117
🏁 Script executed:
cat -n package.json | grep -A 5 -B 5 viemRepository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 440
🏁 Script executed:
cat -n src/utils/transactionHelpers.ts | sed -n '25,33p'Repository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 403
🏁 Script executed:
python3 << 'EOF'
# Simulating the control flow of validateTransactionParams
print("=== CONTROL FLOW ANALYSIS ===\n")
print("Scenario 1: params.amount = 'abc', allowance and balance defined")
print("-" * 60)
print("Line 106: if (!params.amount || !isValidAmount(params.amount))")
print(" - params.amount = 'abc' exists (not falsy)")
print(" - isValidAmount('abc') checks: isNaN(Number('abc')) = true")
print(" - Returns false, so !false = true")
print(" - Error 'Invalid amount' is pushed")
print(" - BUT execution continues!\n")
print("Line 118: if (params.allowance !== undefined && params.balance !== undefined)")
print(" - Both defined, condition is TRUE")
print(" - ENTERS block\n")
print("Line 119: const amountBig = parseUnits(params.amount || '0', 18)")
print(" - params.amount = 'abc' exists (truthy)")
print(" - Calls parseUnits('abc', 18)")
print(" - parseUnits from viem will THROW on invalid numeric input")
print(" - CRASH! Exception is not caught\n")
print("=== CONCLUSION ===")
print("✓ CRASH RISK CONFIRMED")
print(" The function violates the 'collect errors' design pattern")
print(" by allowing parseUnits to throw instead of collecting errors\n")
print("=== HARDCODED DECIMALS ISSUE ===")
print("-" * 60)
print("Line 106: isValidAmount(params.amount) - called WITHOUT decimals")
print(" - Uses default decimals = 18\n")
print("Line 119: parseUnits(params.amount || '0', 18) - hardcoded 18")
print(" - Function doesn't accept decimals parameter")
print(" - Cannot validate/parse with different decimal values")
print("\n✓ HARDCODED DECIMALS CONFIRMED")
EOFRepository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 176
🏁 Script executed:
cat -n src/utils/transactionHelpers.ts | sed -n '140,147p'Repository: DjedAlliance/Djed-Solidity-ERC20BaseCoin-WebUI
Length of output: 372
Line 119 will throw if amount validation fails, crashing the validation function.
The validation check at line 106 correctly identifies invalid amounts but doesn't prevent line 119 from attempting to parse them. If params.amount is invalid (e.g., 'abc') and allowance/balance are defined, parseUnits(params.amount || '0', 18) throws an uncaught exception, violating the function's error-collection design. Additionally, the hardcoded decimal value of 18 is inflexible—the function should accept a decimals parameter like isValidAmount does, especially since tokens can have different decimal places.
export const validateTransactionParams = (params: {
amount?: string;
receiver?: string;
fromAddress?: string;
allowance?: bigint;
balance?: bigint;
}): { isValid: boolean; errors: string[] } => {
const errors: string[] = [];
- if (!params.amount || !isValidAmount(params.amount)) {
+ const amountOk = !!params.amount && isValidAmount(params.amount);
+ if (!amountOk) {
errors.push('Invalid amount');
}
@@
- if (params.allowance !== undefined && params.balance !== undefined) {
- const amountBig = parseUnits(params.amount || '0', 18);
+ if (amountOk && params.allowance !== undefined && params.balance !== undefined) {
+ const amountBig = parseUnits(params.amount!, 18);
if (params.balance < amountBig) {
errors.push('Insufficient balance');
}
if (params.allowance < amountBig) {
errors.push('Insufficient allowance');
}
}📝 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.
| export const validateTransactionParams = (params: { | |
| amount?: string; | |
| receiver?: string; | |
| fromAddress?: string; | |
| allowance?: bigint; | |
| balance?: bigint; | |
| }): { isValid: boolean; errors: string[] } => { | |
| const errors: string[] = []; | |
| if (!params.amount || !isValidAmount(params.amount)) { | |
| errors.push('Invalid amount'); | |
| } | |
| if (params.receiver && !isValidAddress(params.receiver)) { | |
| errors.push('Invalid receiver address'); | |
| } | |
| if (params.fromAddress && !isValidAddress(params.fromAddress)) { | |
| errors.push('Invalid from address'); | |
| } | |
| if (params.allowance !== undefined && params.balance !== undefined) { | |
| const amountBig = parseUnits(params.amount || '0', 18); | |
| if (params.balance < amountBig) { | |
| errors.push('Insufficient balance'); | |
| } | |
| if (params.allowance < amountBig) { | |
| errors.push('Insufficient allowance'); | |
| } | |
| } | |
| return { | |
| isValid: errors.length === 0, | |
| errors, | |
| }; | |
| }; | |
| export const validateTransactionParams = (params: { | |
| amount?: string; | |
| receiver?: string; | |
| fromAddress?: string; | |
| allowance?: bigint; | |
| balance?: bigint; | |
| }): { isValid: boolean; errors: string[] } => { | |
| const errors: string[] = []; | |
| const amountOk = !!params.amount && isValidAmount(params.amount); | |
| if (!amountOk) { | |
| errors.push('Invalid amount'); | |
| } | |
| if (params.receiver && !isValidAddress(params.receiver)) { | |
| errors.push('Invalid receiver address'); | |
| } | |
| if (params.fromAddress && !isValidAddress(params.fromAddress)) { | |
| errors.push('Invalid from address'); | |
| } | |
| if (amountOk && params.allowance !== undefined && params.balance !== undefined) { | |
| const amountBig = parseUnits(params.amount!, 18); | |
| if (params.balance < amountBig) { | |
| errors.push('Insufficient balance'); | |
| } | |
| if (params.allowance < amountBig) { | |
| errors.push('Insufficient allowance'); | |
| } | |
| } | |
| return { | |
| isValid: errors.length === 0, | |
| errors, | |
| }; | |
| }; |
|
@dark-Invincible please include video or ss and resolve all the actionable comments by coderabbit |
|
Thanks for the submission. I’m not approving this PR. The changes appear largely AI-generated without sufficient manual review, and multiple redundant/unnecessary files were added. We expect contributors to understand and validate what they submit. This PR does not meet that standard. |
Resolved #5
Summary by CodeRabbit
Release Notes
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.