Refactor Dashboard, Treasury Account Setup, and Routing #6
Conversation
WalkthroughThe frontend application underwent significant updates focusing on UI/UX enhancements, expanded routing, and integration with Lit Protocol for encryption and decryption of invoice data. Several new dependencies were added for animation, notifications, and blockchain features. Multiple components were refactored for improved layout, error handling, and user feedback, especially around invoice management and treasury operations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant LitNodeClient
participant SmartContract
User->>App: Navigates to SentInvoice or ReceivedInvoice
App->>LitNodeClient: Initialize and connect client
App->>SmartContract: Fetch encrypted invoices
App->>LitNodeClient: Request session signature (SIWE)
App->>LitNodeClient: Decrypt invoice data (with access control)
LitNodeClient-->>App: Return decrypted invoice
App-->>User: Display decrypted invoice in UI
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (17)
.gitmodules (1)
1-6: Pin the submodule to a specific commit or branch for reproducible clones
.gitmodulesonly records the URL/path; the actual revision is tracked in the super-project’s index.
Right now anyone cloning the repo will get whatever the current default branch HEAD is for each upstream, which could break builds if those projects force-push or introduce breaking changes. Add an explicitbranch = <name>(or, better, commit-hash pin viagit submodule update --init --recursive) so CI and contributors all fetch the exact same revision.[submodule "contracts/lib/forge-std"] path = contracts/lib/forge-std url = https://github.com/foundry-rs/forge-std + # Pin to avoid unexpected upstream changes + branch = mainDo the same for
chainlink-brownie-contracts, or commit-pin both in the super-project before merging.frontend/package.json (2)
15-18: Lit SDK: confirm you need four separate packages; consider the umbrella@lit-protocol/sdkThe individual helpers inflate the bundle and duplicate transitive deps (~250 kB min+gz). If you only require encryption & auth, the monorepo now exposes
@lit-protocol/sdkwhich tree-shakes better and simplifies upgrades:- "@lit-protocol/auth-helpers": "^7.2.0", - "@lit-protocol/constants": "^7.2.0", - "@lit-protocol/encryption": "^7.2.0", - "@lit-protocol/lit-node-client": "^7.2.0", + "@lit-protocol/sdk": "^7.2.0",Please double-check feature coverage; this swap could drop ~100 kB from the final chunk.
41-42: Heavyweight icons & toasts – watch the bundle size
react-icons(~800 kB un-gzipped) andreact-hot-toastboth pull inreact/jsx-runtime. Consider:
- Importing only the needed icon modules (
import {FiSend} from 'react-icons/fi') – not wildcard paths.- Evaluating lighter alternatives (
lucide-reactis already in the list; duplication?)- Lazy-loading toast logic.
Minor, but worth a look before prod deploy.
frontend/.gitignore (1)
14-14: Generalise env exclusion to cover stage-specific files
/frontend/.envwill ignore only the root file. To catch.env.local,.env.development, etc., widen the pattern:-.env +.env*Prevents accidental commits of stage-specific secrets.
contracts/lib/forge-std (1)
1-1: Pin submodule to a stable, tagged release & document update workflowThe pointer is set directly to commit
3b20d60d14b3….
While that guarantees immutability, it also means future updates require manual commit-hash edits and obscures which upstream version you’re on. Consider:
- Switching to a signed, semver tag from
foundry-rs/forge-stdto make provenance clearer and upgrades easier.- Adding a brief note in
CONTRIBUTING.md(or a script in CI) that runs
git submodule update --init --recursiveso new contributors and CI pipelines don’t miss the dependency.- Verifying the submodule’s license compatibility (forge-std is MIT, but double-check if you’re vendoring other crates via the submodule).
Please confirm the above is acceptable or provide rationale for hard-pinning to a commit.
frontend/vite.config.js (1)
4-4: Consider conditional activation ofvite-plugin-node-polyfills
vite-plugin-node-polyfillsdramatically increases the production bundle size. Unless you explicitly depend on Node globals in production, gate the plugin bymode(e.g.if (mode === 'development') …) or passinclude: ['node']to limit injected shims.frontend/.env.example (1)
1-15: Clarify placeholder values
To avoid someone copying the file verbatim, wrap placeholders in angle brackets (e.g.<YOUR_CHAIN_ID>). This tiny change signals “replace me” more explicitly.frontend/src/page/Applayout.jsx (1)
1-1: Header comment & filename are out of sync
// AppLayout.jsvs Applayout.jsx vs componentApplayout. Pick one canonical casing to keep searches and imports predictable.README.md (3)
59-63: Typo & missing article- - Copy all the varible from `contracts/.env.example` to newly created `.env` + - Copy all the variables from `contracts/.env.example` to the newly created `.env`
73-78: Add language tag for fenced code block (MD040)-``` +```bash
88-90: Comma before “so” for clarity- - Restart your frontend development server so the new environment variables are loaded. + - Restart your frontend development server, so the new environment variables are loaded.frontend/src/page/Home.jsx (2)
43-43: Remove unnecessary whitespace JSX expressionsThe
{" "}expressions are unnecessary and can be removed. React already handles whitespace appropriately.- {" "}Also applies to: 53-53, 69-69
76-114: Add accessibility attributes to navigation buttonsThe navigation buttons lack proper accessibility attributes for screen readers.
Add
aria-labelandaria-currentattributes:<ListItemButton onClick={() => navigate(item.route)} selected={location.pathname.includes(item.route)} + aria-label={`Navigate to ${item.text}`} + aria-current={location.pathname.includes(item.route) ? "page" : undefined} sx={{frontend/src/page/Treasure.jsx (1)
58-62: Consider additional treasury address validationWhile checking for a valid Ethereum address is good, consider adding additional validation such as checking if the address is not the zero address or the current contract address.
const handleSetTreasuryAddress = async () => { if (!ethers.isAddress(newTreasuryAddress)) { alert("Please enter a valid Ethereum address"); return; } + if (newTreasuryAddress === ethers.ZeroAddress) { + alert("Treasury address cannot be the zero address"); + return; + } + if (newTreasuryAddress.toLowerCase() === import.meta.env.VITE_CONTRACT_ADDRESS?.toLowerCase()) { + alert("Treasury address cannot be the contract address"); + return; + }frontend/src/components/Navbar.jsx (1)
32-40: Consider checking for existing dashboard routes before auto-navigationThe auto-navigation logic could be improved to avoid unnecessary navigation if the user is already on a valid dashboard route.
if ( address && !hasConnected && !location.pathname.startsWith("/dashboard") ) { + // Only navigate if not already on a valid app route + const validAppRoutes = ["/dashboard", "/treasure"]; + const isOnValidRoute = validAppRoutes.some(route => location.pathname.startsWith(route)); + if (!isOnValidRoute) { navigate("/dashboard/create"); + } setHasConnected(true); }frontend/src/components/CreateInvoice.jsx (1)
146-146: Remove unnecessary toString() call.The
addressfrom wagmi is already a string, so callingtoString()is redundant.-address: account?.address.toString(), +address: account?.address,contracts/src/Chainvoice.sol (1)
38-41: Consider initializing treasury address and documenting the fee.The constructor hardcodes a fee of 0.0005 ether but doesn't initialize the treasury address, which could cause
withdrawFeesto fail until set by the owner.-constructor() { +constructor(address _treasuryAddress) { owner = msg.sender; - fee = 0.0005 ether; + fee = 0.0005 ether; // Approximately $1-2 USD at current ETH prices + require(_treasuryAddress != address(0), "Invalid treasury address"); + treasuryAddress = _treasuryAddress; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/public/logo.pngis excluded by!**/*.png
📒 Files selected for processing (21)
.gitmodules(1 hunks)README.md(1 hunks)contracts/.env.example(1 hunks)contracts/lib/forge-std(1 hunks)contracts/src/Chainvoice.sol(1 hunks)contracts/src/MockV3Aggregator.sol(0 hunks)frontend/.env(0 hunks)frontend/.env.example(1 hunks)frontend/.gitignore(2 hunks)frontend/package.json(2 hunks)frontend/src/App.jsx(1 hunks)frontend/src/components/CreateInvoice.jsx(4 hunks)frontend/src/components/Navbar.jsx(1 hunks)frontend/src/contractsABI/ChainvoiceABI.js(1 hunks)frontend/src/page/Applayout.jsx(1 hunks)frontend/src/page/Home.jsx(1 hunks)frontend/src/page/Landing.jsx(1 hunks)frontend/src/page/ReceivedInvoice.jsx(8 hunks)frontend/src/page/SentInvoice.jsx(5 hunks)frontend/src/page/Treasure.jsx(2 hunks)frontend/vite.config.js(1 hunks)
💤 Files with no reviewable changes (2)
- frontend/.env
- contracts/src/MockV3Aggregator.sol
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/components/Navbar.jsx (3)
frontend/src/components/CreateInvoice.jsx (2)
useAccount(30-30)navigate(35-35)frontend/src/page/About.jsx (1)
useAccount(7-7)frontend/src/page/Home.jsx (2)
navigate(16-16)location(17-17)
🪛 LanguageTool
README.md
[typographical] ~59-~59: Consider putting a comma before the abbreviation “i.e.”.
Context: ...ts - Create a .env in your project root i.e. contracts/ - Copy all the varible...
(IE_COMMA)
[uncategorized] ~60-~60: You might be missing the article “the” here.
Context: ...arible from contracts/.env.example to newly created .env `cp .env.ex...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~79-~79: Possible missing comma found.
Context: ...key $PRIVATE_KEY \ --broadcast ``` 5. Finally add Contract Address to Frontend .env...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~79-~79: You might be missing the article “the” here.
Context: ..._KEY \ --broadcast ``` 5. Finally add Contract Address to Frontend .env - Creat...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~90-~90: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...Restart your frontend development server so the new environment variables are loade...
(COMMA_COMPOUND_SENTENCE_2)
🪛 markdownlint-cli2 (0.17.2)
README.md
73-73: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Biome (1.9.4)
frontend/src/page/ReceivedInvoice.jsx
[error] 567-567: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (12)
frontend/package.json (1)
33-33: Verify Framer-Motion v12 ↔ React 18 compatibilityFramer-Motion 12 still marks React 18 support as experimental; there have been breaking changes around
AnimatePresence. Smoke-test interactive pages and pin to an exact minor (12.23.0→12.23.0) if you hit reconciliation warnings.contracts/.env.example (1)
1-12: Verify.envis ignored to prevent key leakage
An example file is great, but please confirm that the realcontracts/.envis excluded via a repo-level orcontracts/.gitignorerule; accidental pushes ofPRIVATE_KEYhappen surprisingly often.frontend/src/page/Applayout.jsx (1)
8-16: Layout refactor looks solid
Semantic<main>element, full-height flex, and spacing all look correct. Nice cleanup.frontend/src/page/Home.jsx (1)
1-142: Well-structured sidebar navigation implementationThe refactored Home component provides a clean and intuitive navigation structure with good visual feedback and responsive design. The use of
useLocationfor active route detection and the enhanced styling improve the user experience.frontend/src/page/Landing.jsx (1)
13-15: Verify if commenting out the redirect is intentionalThe automatic redirect on wallet connection has been disabled. Please confirm this is intentional and aligns with the new user flow.
If this is intentional, consider removing the commented code entirely to avoid confusion.
frontend/src/App.jsx (1)
56-65: Well-organized routing structureThe routing restructure with dashboard as a parent route and nested invoice-related routes provides a clean hierarchy. The addition of new pages and the Toaster component enhance the application's functionality.
frontend/src/page/Treasure.jsx (1)
20-108: Excellent state management and error handling improvementsThe refactoring to use separate loading states and improved error handling with user alerts significantly enhances the user experience. The concurrent data fetching with Promise.all is also more efficient.
frontend/src/components/Navbar.jsx (1)
114-233: Excellent navbar enhancement with animations and responsive designThe comprehensive refactoring with framer-motion animations, improved active route detection, and scroll-based styling creates a polished and professional navigation experience. The separation of nav items and app items provides good organization.
frontend/src/page/ReceivedInvoice.jsx (2)
93-100: Good network validation implementation!The network validation ensures users are on the correct chain before attempting to fetch invoices, providing a clear error message when on the wrong network.
231-254: Payment calculation correctly includes fees!The implementation properly calculates the total payment by adding the invoice amount and contract fee, using BigInt for accurate Wei calculations.
frontend/src/components/CreateInvoice.jsx (1)
51-66: Excellent BigInt implementation for precise calculations!The total amount calculation correctly uses BigInt arithmetic to handle Ethereum's 18 decimal precision, avoiding floating-point errors. The division by
parseUnits("1", 18)properly adjusts for the double scaling when multiplying two 18-decimal values.contracts/src/Chainvoice.sol (1)
77-93: Well-implemented payment function with proper checks!The payment function correctly validates the invoice, ensures exact payment including fees, and uses the recommended
callmethod for ETH transfers instead of the deprecatedtransfer.
frontend/src/page/SentInvoice.jsx
Outdated
| {/* {ethers.formatUnits(item.unitPrice)} */} | ||
| {item.unitPrice} | ||
| </td> | ||
| <td className="border p-2">{item.discount.toString()}</td> | ||
| <td className="border p-2">{item.tax.toString()}</td> | ||
| <td className="border p-2"> | ||
| {/* {ethers.formatUnits(item.amount)} */} | ||
| {item.amount} | ||
| </td> | ||
| </tr> | ||
| </tbody> | ||
| ))} | ||
| </table> | ||
| <div className="mt-4 text-xs"> | ||
| <p className="text-right font-semibold">Fee for invoice pay : {ethers.formatUnits(fee)} ETH</p> | ||
| <p className="text-right font-semibold"> Amount: {ethers.formatUnits(drawerState.selectedInvoice.amountDue)} ETH</p> | ||
| <p className="text-right font-semibold"> Total Amount: {ethers.formatUnits(drawerState.selectedInvoice.amountDue + fee)} ETH</p> | ||
| <p className="text-right font-semibold"> | ||
| Fee for invoice pay : {parseFloat(ethers.formatUnits(fee))}{" "} | ||
| ETH | ||
| {/* Fee for invoice pay : {ethers.formatUnits(fee)} ETH */} | ||
| </p> | ||
| <p className="text-right font-semibold"> | ||
| {" "} | ||
| Amount:{" "} | ||
| {/* {ethers.formatUnits(drawerState.selectedInvoice.amountDue)}{" "} */} | ||
| {drawerState.selectedInvoice.amountDue} ETH | ||
| </p> | ||
| <p className="text-right font-semibold"> | ||
| Total Amount:{" "} | ||
| {parseFloat(drawerState.selectedInvoice.amountDue) + | ||
| parseFloat(ethers.formatUnits(fee))}{" "} | ||
| ETH | ||
| </p> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Standardize the approach for displaying ETH values.
The code contains commented-out ethers.formatUnits calls and uses inconsistent formatting. Some values use parseFloat(ethers.formatUnits(fee)) while others display raw values.
Decide on a consistent approach. If invoice amounts are already in ETH (not Wei), remove the commented code. If they're in Wei, use formatUnits consistently:
-{/* {ethers.formatUnits(item.unitPrice)} */}
-{item.unitPrice}
+{item.unitPrice} {/* Already in ETH */}
-{parseFloat(ethers.formatUnits(fee))} ETH
+{ethers.formatUnits(fee)} ETH
-{parseFloat(drawerState.selectedInvoice.amountDue) +
- parseFloat(ethers.formatUnits(fee))} ETH
+{(parseFloat(drawerState.selectedInvoice.amountDue) +
+ parseFloat(ethers.formatUnits(fee))).toFixed(6)} ETH🤖 Prompt for AI Agents
In frontend/src/page/SentInvoice.jsx lines 515 to 545, the display of ETH values
is inconsistent with some values using raw numbers and others using parseFloat
with ethers.formatUnits, plus commented-out code. Determine if the amounts are
in Wei or already in ETH; if in Wei, consistently use ethers.formatUnits to
convert to ETH for all displayed values and remove commented-out code. If
already in ETH, remove all ethers.formatUnits calls and commented code,
displaying raw values uniformly.
| function withdrawFees() external { | ||
| require(treasuryAddress != address(0), "Treasury not set"); | ||
| require(accumulatedFees > 0, "No fees available"); | ||
|
|
||
| uint256 amount = accumulatedFees; | ||
| accumulatedFees = 0; | ||
|
|
||
| (bool success, ) = payable(treasuryAddress).call{value: amount}(""); | ||
| require(success, "Fee withdrawal failed"); | ||
| require(success, "Withdraw failed"); | ||
| } |
There was a problem hiding this comment.
Add access control to withdrawFees function.
The withdrawFees function lacks access control, allowing anyone to trigger fee withdrawal to the treasury address. While funds go to the treasury, this could be exploited for griefing or gas manipulation.
-function withdrawFees() external {
+function withdrawFees() external onlyOwner {
require(treasuryAddress != address(0), "Treasury not set");
require(accumulatedFees > 0, "No fees available");📝 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.
| function withdrawFees() external { | |
| require(treasuryAddress != address(0), "Treasury not set"); | |
| require(accumulatedFees > 0, "No fees available"); | |
| uint256 amount = accumulatedFees; | |
| accumulatedFees = 0; | |
| (bool success, ) = payable(treasuryAddress).call{value: amount}(""); | |
| require(success, "Fee withdrawal failed"); | |
| require(success, "Withdraw failed"); | |
| } | |
| function withdrawFees() external onlyOwner { | |
| require(treasuryAddress != address(0), "Treasury not set"); | |
| require(accumulatedFees > 0, "No fees available"); | |
| uint256 amount = accumulatedFees; | |
| accumulatedFees = 0; | |
| (bool success, ) = payable(treasuryAddress).call{value: amount}(""); | |
| require(success, "Withdraw failed"); | |
| } |
🤖 Prompt for AI Agents
In contracts/src/Chainvoice.sol around lines 133 to 142, the withdrawFees
function currently lacks access control, allowing any user to call it. To fix
this, restrict access by adding a modifier such as onlyOwner or another
appropriate role-based access control mechanism to ensure only authorized
accounts can invoke withdrawFees. This prevents unauthorized or malicious calls
while preserving the intended functionality.
|
@kumawatkaran523 for UI related changes, please attach a screenshot or a short video showing the updates you've made. |
2 participants
Previous
Now
Previous
Now
previous
Now
