Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .github/workflows/smoke.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,21 @@ jobs:

- run: npm ci

# Smoke tests shell out to `node dist/index.js` to exercise the
# compiled CLI — without a build first, that file doesn't exist
# and the `genlayer staking validators` assertion fails with
# MODULE_NOT_FOUND before it can hit the network at all.
- name: Build CLI
run: npm run build

# The staking subcommands need a keystore for the default account
# even for read-only listings (BaseAction loads the account to
# report "your stake"). Create a throwaway one with a known
# password so the subprocess test in tests/smoke.test.ts can run.
- name: Create default keystore for CLI subprocess tests
run: node dist/index.js account create --name default --password ci-smoke-test
env:
CI: true

- name: Run smoke tests
run: npm run test:smoke
8 changes: 4 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
"dotenv": "^17.0.0",
"ethers": "^6.13.4",
"fs-extra": "^11.3.0",
"genlayer-js": "^0.28.5",
"genlayer-js": "^1.0.0",
"inquirer": "^12.0.0",
"keytar": "^7.9.0",
"node-fetch": "^3.0.0",
Expand Down
24 changes: 13 additions & 11 deletions src/commands/account/send.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,20 @@ export class SendAction extends BaseAction {
}

private parseAmount(amount: string): bigint {
// Support "10gen" or "10" (assumes gen) or wei values
const lowerAmount = amount.toLowerCase();
if (lowerAmount.endsWith("gen")) {
const value = lowerAmount.slice(0, -3);
return parseEther(value);
// 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);
Comment on lines +34 to +47
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 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:


🏁 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.

Suggested change
// 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).

}

async execute(options: SendOptions): Promise<void> {
Expand Down
5 changes: 4 additions & 1 deletion tests/smoke.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ import type {Address, GenLayerChain} from "genlayer-js/types";

const CLI = path.resolve(__dirname, "../dist/index.js");

const TIMEOUT = 30_000;
// Testnet validator-list fetches ALL validators + per-validator detail in
// batches; on bradbury/asimov that routinely passes 30s. 90s gives headroom
// without hiding real hangs.
const TIMEOUT = 90_000;

const testnets: {name: string; chain: GenLayerChain}[] = [
{name: "Asimov", chain: testnetAsimov},
Expand Down
Loading