fix: compare LPT allowance and balance in wei#738
Conversation
/api/account-balance returns balance and allowance in wei, but the delegating widget compared them against the human-readable LPT input. Both checks were true for any realistic amount, so wallets holding a partial allowance never saw the approve flow, the bondWithHint simulation reverted, and the Delegate button became a silent no-op. Compare in wei with BigInt, and disable Delegate while the simulation has no config so a failed simulation cannot present as a working button. Fixes #737 Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Fixes the delegating widget’s allowance/balance gating by comparing the user-entered LPT amount against /api/account-balance’s balance/allowance values in wei (instead of comparing raw-wei strings to a human-readable input). Also prevents the Delegate button from appearing clickable when the bondWithHint simulation hasn’t produced a usable config.
Changes:
- Convert the input
amountto wei (parseEther) and compare usingBigIntagainsttokenBalanceandtransferAllowance. - Disable Delegate actions when
bondWithHintConfigis unavailable to avoid “enabled but no-op” clicks after a failed simulation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // `tokenBalance` and `transferAllowance` are wei, so compare in wei. | ||
| const amountWei = useMemo(() => { | ||
| try { | ||
| return amount ? parseEther(amount) : BigInt(0); | ||
| } catch { |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe delegation widget and footer now parse entered amounts into wei, use bigint comparisons for balance and stake checks, and gate simulations and buttons on valid amounts and available configuration. Tests cover decimal, precision, exponent, and invalid input handling. ChangesDelegation amount handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 14 UNAVAILABLE: read ECONNRESET 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/DelegatingWidget/Delegate.tsx`:
- Around line 170-184: Move the safe amountWei useMemo above bondWithHintArgs in
Delegate and update bondWithHintArgs to use amountWei instead of calling
parseEther(amount) directly. Remove the amountWei > BigInt(0) conditions from
sufficientBalance and sufficientTransferAllowance so zero-amount inputs are
evaluated by the balance and allowance comparisons while preserving safe
handling of invalid input.
🪄 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: c0f5740e-1ef5-4b83-97da-b8a516979cad
📒 Files selected for processing (1)
components/DelegatingWidget/Delegate.tsx
parseEther was called unguarded when building bondWithHintArgs, so a value the number input accepts but viem rejects (e.g. `1e3`) threw during render and took the widget down. The bondWithHint simulation also ran before the allowance was in place, and nothing re-runs a cached failure once the approval confirms, leaving Delegate disabled until the amount was retyped or the page reloaded. Parse the amount once, defensively, and reuse it for both the simulation args and the balance/allowance checks. Gate the simulation on the allowance so it first runs when it can succeed; a zero amount moves no tokens and needs none, which keeps "Move Delegated Stake" working. Keeps the amountWei > 0 guards: they match the previous parseFloat(amount) > 0 behaviour, and dropping them would enable Delegate for a zero-amount bond. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Co-Authored-By: Copilot <copilot-pull-request-reviewer[bot]@users.noreply.github.com> Co-Authored-By: CodeRabbit <coderabbitai[bot]@users.noreply.github.com>
parseEther throws on values the amount input accepts but cannot represent as a decimal (e.g. `1e3`). Footer called it unguarded when computing the active set order, so typing one crashed the render. Parse through a shared parseAmountToWei helper that returns null instead of throwing, and treat null as "no amount entered" so the button falls back to its existing disabled state. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The amount input accepts `1e-2`, but parseEther rejects it, so valid amounts were treated as no input at all. Normalise exponent notation to a decimal before parsing; plain decimals skip Number to avoid float precision loss. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
parseAmountToWei had no tests. Pin the decimal, exponent, and null cases, and the precision behaviour that requires exponents alone to route through Number. The boundary test records that exponents outside the range Number writes as a decimal are rejected, while the same value in decimal notation is not. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/DelegatingWidget/Footer.tsx (1)
100-103: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRely on
amountWeiinstead ofparseFloatfor undelegation validation.Problem:
canUndelegateusesparseFloat(amount), which leniently parses partially invalid strings (like"1e"as1). This enables the Undelegate button even whenamountWeicorrectly rejects the amount asnull.
Why it matters: Users can submit unparseable amounts to the underlyingUndelegatecomponent, which will either crash or submit an invalid zero-amount transaction.
Suggested fix: Use the correctly parsedamountWeito determine if the amount is strictly positive and valid.💻 Proposed refactor
const canUndelegate = useMemo( - () => isMyTranscoder && isDelegated && parseFloat(amount) > 0, - [isMyTranscoder, isDelegated, amount] + () => isMyTranscoder && isDelegated && amountWei !== null && amountWei > 0n, + [isMyTranscoder, isDelegated, amountWei] );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/DelegatingWidget/Footer.tsx` around lines 100 - 103, Update the canUndelegate useMemo to validate the parsed amountWei value instead of parseFloat(amount). Require amountWei to be non-null and strictly greater than zero, while preserving the existing isMyTranscoder and isDelegated conditions and dependency tracking.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/DelegatingWidget/Delegate.tsx`:
- Around line 226-232: Update the early-return validation in Delegate so an
unparseable amount cannot reach the stake-transfer execution path: treat
amountWei === null as invalid whenever amount is non-empty, including when
isTransferStake is true. Preserve the existing allowance for intentionally empty
amounts during stake transfers and keep the disabled “Enter an Amount” button
behavior.
---
Outside diff comments:
In `@components/DelegatingWidget/Footer.tsx`:
- Around line 100-103: Update the canUndelegate useMemo to validate the parsed
amountWei value instead of parseFloat(amount). Require amountWei to be non-null
and strictly greater than zero, while preserving the existing isMyTranscoder and
isDelegated conditions and dependency tracking.
🪄 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: 7f77c803-d0dd-46e5-8919-c09b24ab4ace
📒 Files selected for processing (4)
components/DelegatingWidget/Delegate.tsxcomponents/DelegatingWidget/Footer.tsxutils/web3.test.tsutils/web3.ts
amountIsNonEmpty returned the amount string rather than a boolean, and only worked because every use site read it for truthiness. Its last use asks whether there is an amount worth approving for, which amountWei answers directly. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
A stake transfer bonds zero to switch delegate, so the empty-amount branch let any unparseable amount through as well. Entering one would move the delegation without adding the stake that was typed. Allow the fall-through only when the field is genuinely empty. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Co-Authored-By: CodeRabbit <coderabbitai[bot]@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Fixes #737.
Problem
/api/account-balancereturnsbalanceandallowanceas raw wei, butDelegate.tsxcompared them againstamount, the human-readable LPT input:Both are true for any realistic input, so a wallet holding a partial (non-zero but insufficient) allowance never saw the approve flow, the
bondWithHintsimulation reverted,bondWithHintConfigstayed undefined, andonDelegatethrew into aconsole.error— leaving an enabled Delegate button that did nothing. TheInsufficient Balanceguard could never fire either.Only partial allowances are affected: at allowance
0the approve flow shows correctly and grantsMAX_UINT256, which is why the normal path works.Fix
Convert the input with
parseEtherand compare in wei viaBigInt. Also disable the Delegate button whilebondWithHintConfigis undefined, so a failed simulation can no longer present as a working button.Notes
Introduced in 989c09b (#165), which created
/api/account-balanceand rewired both consumers to it —InputBoxkept itsfromWei,Footerdropped it. That split is why the displayed balance stayed correct while the comparison broke.Summary by CodeRabbit
toWeihelper as deprecated in its docs.