fix(account): send parses plain integer as wei, not GEN#294
fix(account): send parses plain integer as wei, not GEN#294MuncleUscles merged 5 commits intomainfrom
Conversation
The old parseAmount had a bias: integers below 10^12 wei were silently interpreted as GEN, integers above as wei. That makes `account send 1000` attempt a 1000 GEN transfer, not 1000 wei — a footgun that's inconsistent with how parseStakingAmount and the SDK's parseStakingAmount handle the same units. Align with the symmetric convention used elsewhere in the codebase: - 'Ngen' / 'N gen' suffix → N GEN via parseEther - plain integer → wei (BigInt, no conversion) - plain decimal without suffix → rejected (ambiguous, was silently GEN for small values and wei for large)
|
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 4 minutes and 11 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: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 1
🤖 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/commands/account/send.ts`:
- Around line 34-47: The amount parser currently lets BigInt(trimmed) accept
signed and base-prefixed values; change the wei branch in send.ts to only accept
an unsigned decimal integer by validating trimmed against a whitelist regex
(e.g., /^[0-9]+$/) and then converting with BigInt, throwing a clear error for
invalid input; keep the existing GEN-suffix handling (parseEther) for decimal
GEN amounts. Also update the help text in account/index.ts to clearly document
the amount grammar: explain that trailing "gen" (case-insensitive) denotes
decimal GEN (supports fractions) and plain unsigned integers (no sign or 0x
prefix) denote wei. Ensure error messages reference the same grammar so users
know to use e.g. "1gen" or "1000" (wei).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 041c10d4-a79d-468d-8a08-aae981df3103
📒 Files selected for processing (1)
src/commands/account/send.ts
| // Symmetric with genlayer-js `parseStakingAmount`: | ||
| // "Ngen" → N GEN (parseEther) | ||
| // integer → wei (as-is) | ||
| // decimal without suffix → rejected (ambiguous; was previously silently | ||
| // interpreted as GEN for small values and as wei for large — a footgun | ||
| // that made `send 1000` attempt to transfer 1000 GEN, not 1000 wei). | ||
| const trimmed = amount.trim(); | ||
| const lower = trimmed.toLowerCase(); | ||
| if (lower.endsWith("gen")) { | ||
| return parseEther(lower.slice(0, -3).trim()); | ||
| } | ||
| // If it's a large number (likely wei), use as-is | ||
| if (BigInt(amount) > 1_000_000_000_000n) { | ||
| return BigInt(amount); | ||
| } | ||
| // Otherwise assume it's in GEN | ||
| return parseEther(amount); | ||
| // Plain integer → wei. BigInt() throws on decimals, which is the intended | ||
| // failure mode for ambiguous input like "1.5". | ||
| return BigInt(trimmed); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does JavaScript BigInt(string) accept signed and base-prefixed string inputs such as "-1" and "0x10", and does viem parseEther accept signed decimal strings?
💡 Result:
Yes, JavaScript's BigInt(string) accepts signed strings like "-1" and base-prefixed strings like "0x10". No, viem's parseEther does not accept signed decimal strings; while its underlying parseUnits supports negatives technically, parseEther is designed for positive ether amounts in Ethereum transactions, and negative values are invalid for such contexts (e.g., uint256 types). Using negatives would produce a negative BigInt, which cannot be encoded as unsigned transaction data.
Citations:
- 1: https://developer.cdn.mozilla.net/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt/BigInt
- 2: https://dev.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt/BigInt
- 3: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt/BigInt
- 4: https://github.com/wagmi-dev/viem/blob/main/src/utils/unit/parseUnits.ts
🏁 Script executed:
# Check the actual content of send.ts around lines 34-47
cat -n src/commands/account/send.ts | sed -n '30,50p'Repository: genlayerlabs/genlayer-cli
Length of output: 1900
🏁 Script executed:
# Check the command definition and help text in index.ts
cat -n src/commands/account/index.ts | sed -n '90,115p'Repository: genlayerlabs/genlayer-cli
Length of output: 1603
🏁 Script executed:
# Also verify that SendAction extends BaseAction as stated
cat -n src/commands/account/send.ts | sed -n '1,25p'Repository: genlayerlabs/genlayer-cli
Length of output: 973
🏁 Script executed:
# Get the actual parseAmount implementation at lines 34-47
cat -n src/commands/account/send.ts | sed -n '34,47p'Repository: genlayerlabs/genlayer-cli
Length of output: 854
Whitelist the amount grammar and update help text.
BigInt(trimmed) accepts signed values (e.g., "-1") and base-prefixed strings (e.g., "0x10"), which contradicts the stated "plain integer → wei" contract. Additionally, the help text at line 99 of src/commands/account/index.ts only says "Send GEN to an address" without documenting that plain integers are wei and the GEN suffix is required for decimal amounts.
Update the validation to explicitly accept only plain positive integers for wei, and update the help text to document the amount format.
🛡️ Proposed validation and help text updates
For src/commands/account/send.ts lines 40–47:
const trimmed = amount.trim();
const lower = trimmed.toLowerCase();
if (lower.endsWith("gen")) {
- return parseEther(lower.slice(0, -3).trim());
+ const genAmount = lower.slice(0, -3).trim();
+ if (!/^\d+(?:\.\d+)?$/.test(genAmount)) {
+ throw new Error("Invalid amount. Use an integer wei value or a decimal GEN value with a GEN suffix.");
+ }
+ return parseEther(genAmount);
}
// Plain integer → wei. BigInt() throws on decimals, which is the intended
// failure mode for ambiguous input like "1.5".
+ if (!/^\d+$/.test(trimmed)) {
+ throw new Error("Invalid amount. Use an integer wei value or add a GEN suffix for GEN amounts.");
+ }
return BigInt(trimmed);For src/commands/account/index.ts line 99:
- .description("Send GEN to an address")
+ .description("Send GEN or wei to an address (amount: integer wei or decimal GEN with 'GEN' suffix)")📝 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.
| // Symmetric with genlayer-js `parseStakingAmount`: | |
| // "Ngen" → N GEN (parseEther) | |
| // integer → wei (as-is) | |
| // decimal without suffix → rejected (ambiguous; was previously silently | |
| // interpreted as GEN for small values and as wei for large — a footgun | |
| // that made `send 1000` attempt to transfer 1000 GEN, not 1000 wei). | |
| const trimmed = amount.trim(); | |
| const lower = trimmed.toLowerCase(); | |
| if (lower.endsWith("gen")) { | |
| return parseEther(lower.slice(0, -3).trim()); | |
| } | |
| // If it's a large number (likely wei), use as-is | |
| if (BigInt(amount) > 1_000_000_000_000n) { | |
| return BigInt(amount); | |
| } | |
| // Otherwise assume it's in GEN | |
| return parseEther(amount); | |
| // Plain integer → wei. BigInt() throws on decimals, which is the intended | |
| // failure mode for ambiguous input like "1.5". | |
| return BigInt(trimmed); | |
| // Symmetric with genlayer-js `parseStakingAmount`: | |
| // "Ngen" → N GEN (parseEther) | |
| // integer → wei (as-is) | |
| // decimal without suffix → rejected (ambiguous; was previously silently | |
| // interpreted as GEN for small values and as wei for large — a footgun | |
| // that made `send 1000` attempt to transfer 1000 GEN, not 1000 wei). | |
| const trimmed = amount.trim(); | |
| const lower = trimmed.toLowerCase(); | |
| if (lower.endsWith("gen")) { | |
| const genAmount = lower.slice(0, -3).trim(); | |
| if (!/^\d+(?:\.\d+)?$/.test(genAmount)) { | |
| throw new Error("Invalid amount. Use an integer wei value or a decimal GEN value with a GEN suffix."); | |
| } | |
| return parseEther(genAmount); | |
| } | |
| // Plain integer → wei. BigInt() throws on decimals, which is the intended | |
| // failure mode for ambiguous input like "1.5". | |
| if (!/^\d+$/.test(trimmed)) { | |
| throw new Error("Invalid amount. Use an integer wei value or add a GEN suffix for GEN amounts."); | |
| } | |
| return BigInt(trimmed); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/commands/account/send.ts` around lines 34 - 47, The amount parser
currently lets BigInt(trimmed) accept signed and base-prefixed values; change
the wei branch in send.ts to only accept an unsigned decimal integer by
validating trimmed against a whitelist regex (e.g., /^[0-9]+$/) and then
converting with BigInt, throwing a clear error for invalid input; keep the
existing GEN-suffix handling (parseEther) for decimal GEN amounts. Also update
the help text in account/index.ts to clearly document the amount grammar:
explain that trailing "gen" (case-insensitive) denotes decimal GEN (supports
fractions) and plain unsigned integers (no sign or 0x prefix) denote wei. Ensure
error messages reference the same grammar so users know to use e.g. "1gen" or
"1000" (wei).
Picks up the validatorDeposit/Exit routing fix (genlayer-js#155) which routes those two calls through the ValidatorWallet so Staking sees the wallet as msg.sender (previously reverted). The CLI's own staking subcommands already route through VALIDATOR_WALLET_ABI directly (walletClient.writeContract on the validator address), so no call-site changes are needed — this is just keeping the bundled SDK current.
The smoke suite shells out to `node dist/index.js` to exercise the compiled CLI end-to-end (the `genlayer staking validators` assertion in tests/smoke.test.ts). Without a build step that entry point doesn't exist and the test fails with MODULE_NOT_FOUND before it can hit the network. Pre-existing bug — every PR has been red on this check; it's only green on main because the scheduled run happens after a release built dist/.
1 participant
Summary
Test plan
Summary by CodeRabbit
New Features
Bug Fixes