Repository navigation
Fix asset burn max - #507
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 6 minutes and 22 seconds. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAsset amount handling changed from strings to numbers across asset screens; Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
Deploying wallet-mutinynet with
|
| Latest commit: |
ebac595
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d491a181.arkade-wallet.pages.dev |
| Branch Preview URL: | https://fix-asset-ui.arkade-wallet.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/Input.tsx`:
- Line 19: The value/onChange mismatch: update handleInput (the input change
handler) to preserve numeric types for number inputs or when the incoming prop
value is a number — detect input type or whether props.value is a number and
convert e.target.value to Number before calling onChange; ensure the onChange
prop signature in the component props (and any usages like setAmount in
Burn.tsx) expects number | string consistently, or prefer converting to number
for numeric inputs so setAmount receives a number. Locate the Input component
(props.value and handleInput) and adjust the conversion logic and prop types
accordingly.
In `@src/screens/Apps/Assets/Burn.tsx`:
- Line 28: The numeric state "amount" is declared as number and relies on the
Input component to pass numbers, but Input.handleInput currently forwards raw
string values; update Input.handleInput (and any related handler in Input.tsx)
to detect when the input's type is 'number' or when the value can be parsed as a
finite number and call onChange with Number(value) (or parsed numeric value)
instead of the raw string so that setAmount receives a number; ensure this
behavior triggers for type='number' props and preserve existing behavior for
non-numeric inputs.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ae642c97-956a-4727-b97c-fd19e97aa928
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
src/components/AssetCard.tsxsrc/components/Input.tsxsrc/screens/Apps/Assets/Burn.tsx
🔍 Arkana PR Review — wallet#507Fix asset burn max | 4 files changed | +16 / -15 SummaryFixes the asset burn screen: the "max" button now works correctly, amounts display with proper formatting, and the input type is updated. Changes Breakdown
Review
SecurityNo security concerns — UI-only changes to asset burn display and input handling. No transaction signing, key, or VTXO logic affected. The burn amount still goes through VerdictThe core fix is correct. The type flow concern in point #2 is worth a quick check — if |
Deploying wallet-bitcoin with
|
| Latest commit: |
ebac595
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4d586c30.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://fix-asset-ui.wallet-bitcoin.pages.dev |
🔍 Arkana PR ReviewChange: Fixes asset burn/mint/reissue flows to use numeric state and proper formatting for asset amounts. Files changed: Key Changes
Observations
onChange={(v: number) => {
if (!v && v !== 0) {The
✅ No security concerns — changes are UI-only (formatting/display), no transaction logic or key handling affected. The ✅ Overall a solid cleanup. The two items above are minor but worth confirming. Automated review by Arkana |
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)
src/screens/Apps/Assets/Mint.tsx (1)
173-188:⚠️ Potential issue | 🟠 MajorValidate
decimalswithNumber, notparseInt.
parseInt('1.5')becomes1, so a fractional entry like1.5passes this check and later mints with 1 decimal instead of being rejected. UseNumber(decimals)plusNumber.isInteger(...)here, and mirror the same change inhandleMinton Lines 90-92.Suggested fix
- const parsedDecimals = decimals !== '' ? parseInt(decimals) : NaN + const parsedDecimals = decimals !== '' ? Number(decimals) : NaN const disabledReason = !name ? 'Enter a name' : name.length > 40 ? 'Name must be 40 characters or less' : !ticker ? 'Enter a ticker' : ticker.length > 8 ? 'Ticker must be 8 characters or less' : !amount ? 'Enter an amount' : isNaN(parsedUnits) || parsedUnits <= 0 ? 'Amount must be a positive number' - : isNaN(parsedDecimals) || parsedDecimals < 0 || parsedDecimals > 8 + : !Number.isInteger(parsedDecimals) || parsedDecimals < 0 || parsedDecimals > 8 ? 'Decimals must be 0–8' : controlMode === 'New' && !ctrlAmount ? 'Enter control asset amount'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/Apps/Assets/Mint.tsx` around lines 173 - 188, The decimals validation uses parseInt which accepts fractional strings like "1.5"; update the validation to use Number(decimals) and Number.isInteger to ensure decimals is an integer within 0–8 (replace parsedDecimals = decimals !== '' ? parseInt(decimals) : NaN with using Number and Number.isInteger), and make the identical change in the handleMint function (the logic around parsedDecimals in handleMint) so both the UI disabledReason check and the mint execution reject fractional decimal inputs.
🤖 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/Input.tsx`:
- Around line 48-50: The handler handleInput currently coerces number inputs
with Number(v) which turns an empty string into 0 and loses edit-state; change
handleInput to pass the raw string value for type === 'number' (i.e., call
onChange with the uncoerced v for number inputs) so callers can distinguish ""
from 0, and move normalization/parsing (Number(...) and validation) into the
screen-level submit/validation logic (where you should handle conversion from
the raw string to a numeric value and return validation errors as needed).
---
Outside diff comments:
In `@src/screens/Apps/Assets/Mint.tsx`:
- Around line 173-188: The decimals validation uses parseInt which accepts
fractional strings like "1.5"; update the validation to use Number(decimals) and
Number.isInteger to ensure decimals is an integer within 0–8 (replace
parsedDecimals = decimals !== '' ? parseInt(decimals) : NaN with using Number
and Number.isInteger), and make the identical change in the handleMint function
(the logic around parsedDecimals in handleMint) so both the UI disabledReason
check and the mint execution reject fractional decimal inputs.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b5dfea7e-0286-4ef3-b15d-78de03ed683e
📒 Files selected for processing (3)
src/components/Input.tsxsrc/screens/Apps/Assets/Mint.tsxsrc/screens/Apps/Assets/Reissue.tsx
🔍 Arkana Review — Followup on
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/screens/Apps/Assets/Mint.tsx`:
- Line 115: The code forwards ctrlAmount into ctrlRawAmount without ensuring
it's an integer despite the asset metadata using decimals: 0; update the
validation where ctrlRawAmount is used (and where you currently check
isNaN/positive) to require an integer by using
Number.isInteger(Number(ctrlAmount)) or parsing to a Number and verifying value
=== Math.floor(value) and > 0, and surface a validation error if it fails;
additionally, update the control amount input tied to ctrlAmount (the input
rendering logic) to include step="1" and inputMode="numeric" to prevent/limit
fractional entry at the UI level.
- Around line 173-174: The decimals field currently uses parseInt(decimals)
(assigned to parsedDecimals) which silently truncates floats; update the
decimals input onChange handler to validate using
Number.isInteger(Number(value)) and reject or prevent non-integer values (e.g.,
show validation error/ignore change) and replace parseInt(decimals) with
Number(decimals) so invalid (non-integer/NaN) values are caught by that
validation; apply the same change where decimals is parsed earlier (the other
parseInt usage) and ensure any logic that uses parsedDecimals checks
Number.isInteger(parsedDecimals) or NaN handling before proceeding with minting.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 47dd3d46-cc48-4d58-97ca-da8bd93ca0cc
📒 Files selected for processing (1)
src/screens/Apps/Assets/Mint.tsx
🔍 Arkana Review — Followup on
|
Closes #376
@pietro909 please review
Summary by CodeRabbit