Logic checks ethBalance < BigInt(fee) ignores gas costs#126
Logic checks ethBalance < BigInt(fee) ignores gas costs#126aniket866 wants to merge 2 commits intoStabilityNexus:mainfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughA gasBuffer of 0.002 ETH is introduced to balance checks in the ReceivedInvoice component, addressing cases where ETH and ERC20 payments were insufficient to account for transaction gas costs. Error messages are updated to reflect required amounts including gas. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/page/ReceivedInvoice.jsx (1)
1-11:⚠️ Potential issue | 🟡 MinorMissing "use client" directive for Next.js.
This component uses client-side React hooks (
useState,useEffect) and wallet client interactions via wagmi. Per Next.js best practices, add the "use client" directive at the top of the file.Proposed fix
+"use client"; + import Paper from "@mui/material/Paper"; import Table from "@mui/material/Table";As per coding guidelines:
**/*.{ts,tsx,js,jsx}: NextJS - Ensure that "use client" is being used.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/page/ReceivedInvoice.jsx` around lines 1 - 11, This file (ReceivedInvoice.jsx) uses client-side React hooks (useState, useEffect) and wallet interactions (BrowserProvider, Contract, ethers, ChainvoiceABI) but is missing Next.js's "use client" directive; fix by adding the "use client" directive as the very first line of the file so the component runs on the client (ensure it appears before any imports and then keep the existing imports and component code unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/page/ReceivedInvoice.jsx`:
- Around line 240-256: Replace the hardcoded gasBuffer in ReceivedInvoice.jsx
with a configurable, chain-aware value (e.g., use a GAS_BUFFER_BY_CHAIN map
keyed by chainId or an env-configurable fallback) and compute gasBuffer =
GAS_BUFFER_BY_CHAIN[chainId] || GAS_BUFFER_BY_CHAIN.default; update the
totalRequired calculation to use that gasBuffer; and externalize the thrown
error string into the i18n resource (use an i18n key like
"errors.insufficient_eth" and format in the message with the required/available
amounts) so user-visible text is not hardcoded.
- Around line 246-257: The totalRequired calculation for native ETH currently
uses a single fee; update it to include the batch fee by adding fee multiplied
by invoiceCount (e.g., BigInt(fee) * BigInt(invoiceCount)) so types align with
ethers.parseUnits(amount.toString(), 18) which returns a BigInt; modify the
expression assigned to totalRequired (and any related comparisons against
balance) to use parsed amount + BigInt(fee) * BigInt(invoiceCount) + gasBuffer
so batch ETH payments are correctly covered.
- Around line 271-282: The balance check currently only uses a single `fee` when
computing `totalEthRequired`; update the logic (in the function that contains
`ethBalance`, `fee`, `gasBuffer` — e.g., `checkBalance`) to accept an
`invoiceCount` (or compute `totalFee = BigInt(fee) * BigInt(invoiceCount)`) and
use `totalFee + gasBuffer` for `totalEthRequired`; also adjust the error message
to show the required ETH based on `totalFee` and ensure the batch payment call
(the code path referenced at the later batch payment around line with
`invoiceCount` / totalFee) passes the `invoiceCount` into this check so batch
payments are validated correctly before attempting the transaction.
---
Outside diff comments:
In `@frontend/src/page/ReceivedInvoice.jsx`:
- Around line 1-11: This file (ReceivedInvoice.jsx) uses client-side React hooks
(useState, useEffect) and wallet interactions (BrowserProvider, Contract,
ethers, ChainvoiceABI) but is missing Next.js's "use client" directive; fix by
adding the "use client" directive as the very first line of the file so the
component runs on the client (ensure it appears before any imports and then keep
the existing imports and component code unchanged).
|
@kumawatkaran523 @DengreSarthak @aniket866 both #126 and #124 address the same issue #123, I guess he made two by mistake. @aniket866 I feel you should close one from the two |
2 participants
Addressed Issues:
Fixes #123
Description
Where? ReceivedInvoice.jsx
Issue: checkBalance logic for fees.
- Fix: Logic checks ethBalance < BigInt(fee) but ignores gas costs.
Screenshots/Recordings:
Additional Notes:
Checklist
AI Usage Disclosure
Check one of the checkboxes below:
I have used the following AI models and tools: TODO
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact.
Summary by CodeRabbit